added LDAP group name label in add account wizard

changed the parameter for domain in api importLdapUser from name to UUID

improved error handling

client/WEB-INF/classes/resources/messages.properties

@@ -1250,6 +1250,7 @@ label.zoneWizard.trafficType.guest=Guest\: Traffic between end-user virtual mach
 label.zoneWizard.trafficType.management=Management\: Traffic between CloudStack\\\\'s internal resources, including any components that communicate with the Management Server, such as hosts and CloudStack system VMs
 label.zoneWizard.trafficType.public=Public\: Traffic between the internet and virtual machines in the cloud.
 label.zoneWizard.trafficType.storage=Storage\: Traffic between primary and secondary storage servers, such as VM templates and snapshots
+label.ldap.group.name=LDAP Group
 managed.state=Managed State
 message.acquire.new.ip.vpc=Please confirm that you would like to acquire a new IP for this VPC.
 message.acquire.new.ip=Please confirm that you would like to acquire a new IP for this network.

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

@@ -16,11 +16,17 @@
 // under the License.
 package org.apache.cloudstack.api.command;
 
-import com.cloud.domain.Domain;
+import java.security.NoSuchAlgorithmException;
-import com.cloud.exception.*;
+import java.security.SecureRandom;
-import com.cloud.user.AccountService;
+import java.util.ArrayList;
-import com.cloud.user.DomainService;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+
+import javax.inject.Inject;
+
 import org.apache.cloudstack.api.*;
+import org.apache.cloudstack.api.response.DomainResponse;
 import org.apache.cloudstack.api.response.LdapUserResponse;
 import org.apache.cloudstack.api.response.ListResponse;
 import org.apache.cloudstack.ldap.LdapManager;
@@ -30,13 +36,10 @@ import org.apache.commons.lang.StringUtils;
 import org.apache.log4j.Logger;
 import org.bouncycastle.util.encoders.Base64;
 
-import javax.inject.Inject;
+import com.cloud.domain.Domain;
-import java.security.NoSuchAlgorithmException;
+import com.cloud.exception.*;
-import java.security.SecureRandom;
+import com.cloud.user.AccountService;
-import java.util.ArrayList;
+import com.cloud.user.DomainService;
-import java.util.List;
-import java.util.Map;
-import java.util.UUID;
 
 @APICommand(name = "importLdapUsers", description = "Import LDAP users", responseObject = LdapUserResponse.class, since = "4.3.0")
 public class LdapImportUsersCmd extends BaseListCmd {
@@ -45,26 +48,26 @@ public class LdapImportUsersCmd extends BaseListCmd {
 
     private static final String s_name = "ldapuserresponse";
 
-    @Parameter(name = ApiConstants.TIMEZONE, type = CommandType.STRING,
+    @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.")
-	       description = "Specifies a timezone for this command. For more information on the timezone parameter, see Time Zone Format.")
     private String timezone;
 
-    @Parameter(name = ApiConstants.ACCOUNT_TYPE, type = CommandType.SHORT, required = true,
+    @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")
-	       description = "Type of the account.  Specify 0 for user, 1 for root admin, and 2 for domain admin")
     private Short accountType;
 
     @Parameter(name = ApiConstants.ACCOUNT_DETAILS, type = CommandType.MAP, description = "details for account used to store specific parameters")
     private Map<String, String> details;
 
-    @Parameter(name = ApiConstants.DOMAIN, type = CommandType.STRING,
+    @Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, entityType = DomainResponse.class, description = "Specifies the domain to which the ldap users are to be "
-	       description = "Specifies the domain to which the ldap users are to be imported. If no domain is specified, a domain will created using group parameter. If the " +
+            + "imported. If no domain is specified, a domain will created using group parameter. If the group is also not specified, a domain name based on the OU information will be "
-		   "group is also not specified, a domain name based on the OU information will be created. If no OU hierarchy exists, will be defaulted to ROOT domain")
+            + "created. If no OU hierarchy exists, will be defaulted to ROOT domain")
-    private String domainName;
+    private Long domainId;
 
-    @Parameter(name = ApiConstants.GROUP, type = CommandType.STRING,
+    @Parameter(name = ApiConstants.GROUP, type = CommandType.STRING, description = "Specifies the group name from which the ldap users are to be imported. "
-	       description = "Specifies the group name from which the ldap users are to be imported. If no group is specified, all the users will be imported.")
+            + "If no group is specified, all the users will be imported.")
     private String groupName;
 
+    private Domain _domain;
+
     @Inject
     private LdapManager _ldapManager;
 
@@ -82,54 +85,73 @@ public class LdapImportUsersCmd extends BaseListCmd {
     @Override
     public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException, ResourceAllocationException,
             NetworkRuleConflictException {
-	List<LdapUserResponse> ldapResponses = null;
+
-	final ListResponse<LdapUserResponse> response = new ListResponse<LdapUserResponse>();
-	try {
         List<LdapUser> users;
-	if(StringUtils.isNotBlank(groupName)) {
+        try {
+            if (StringUtils.isNotBlank(groupName)) {
                 users = _ldapManager.getUsersInGroup(groupName);
             } else {
                 users = _ldapManager.getUsers();
             }
+        } catch (NoLdapUserMatchingQueryException ex) {
+            users = new ArrayList<LdapUser>();
+            s_logger.info("No Ldap user matching query. "+" ::: "+ex.getMessage());
+        }
+
+        List<LdapUser> addedUsers = new ArrayList<LdapUser>();
         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());
+                addedUsers.add(user);
+            } catch (InvalidParameterValueException ex) {
+                s_logger.error("Failed to create user with username: " + user.getUsername() +" ::: "+ex.getMessage());
             }
-	    ldapResponses = createLdapUserResponse(users);
+        }
-	} catch (final NoLdapUserMatchingQueryException ex) {
+        ListResponse<LdapUserResponse> response = new ListResponse<LdapUserResponse>();
-	    ldapResponses = new ArrayList<LdapUserResponse>();
+        response.setResponses(createLdapUserResponse(addedUsers));
-	} finally {
-	    response.setResponses(ldapResponses);
         response.setResponseName(getCommandName());
         setResponseObject(response);
     }
+
+    private Domain getDomainForName(String name) {
+        Domain domain = null;
+        if (StringUtils.isNotBlank(name)) {
+            //removing all the special characters and trimming its length to 190 to make the domain valid.
+            String domainName = StringUtils.substring(name.replaceAll("\\W", ""), 0, 190);
+            if (StringUtils.isNotBlank(domainName)) {
+                domain = _domainService.getDomainByName(domainName, Domain.ROOT_DOMAIN);
+                if (domain == null) {
+                    domain = _domainService.createDomain(domainName, Domain.ROOT_DOMAIN, domainName, UUID.randomUUID().toString());
+                }
+            }
+        }
+        return domain;
     }
 
     private Domain getDomain(LdapUser user) {
-	String csDomainName = null;
-	if (StringUtils.isNotBlank(domainName)) {
-	    csDomainName = domainName;
-	} else {
-	    if (StringUtils.isNotBlank(groupName)) {
-		csDomainName = groupName;
-	    } else if (StringUtils.isNotBlank(user.getDomain())) {
-		csDomainName = user.getDomain();
-	    }
-	    //removing all the special characters and trimming it length 190 to make the domain valid.
-	    csDomainName = StringUtils.substring(csDomainName.replaceAll("\\W",""),0,190);
-	}
         Domain domain;
-	if (StringUtils.isNotBlank(csDomainName)) {
+        if (_domain != null) {
-	    domain = _domainService.getDomainByName(csDomainName, Domain.ROOT_DOMAIN);
+            //this means either domain id or groupname is passed and this will be same for all the users in this call. hence returning it.
-
+            domain = _domain;
-	    if (domain == null) {
-		domain = _domainService.createDomain(csDomainName, Domain.ROOT_DOMAIN, csDomainName, UUID.randomUUID().toString());
-	    }
         } else {
+            if (domainId != null) {
+                // a domain Id is passed. use it for this user and all the users in the same api call (by setting _domain)
+                domain = _domain = _domainService.getDomain(domainId);
+            } else {
+                // a group name is passed. use it for this user and all the users in the same api call(by setting _domain)
+                domain = _domain = getDomainForName(groupName);
+                if (domain == null) {
+                    //use the domain from the LDAP for this user
+                    domain = getDomainForName(user.getDomain());
+                }
+            }
+            if (domain == null) {
+                // could not get a domain using domainId / LDAP group / OU of LDAP user. using ROOT domain for this user
                 domain = _domainService.getDomain(Domain.ROOT_DOMAIN);
             }
-
+        }
         return domain;
     }
 

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

@@ -20,8 +20,6 @@ import com.cloud.domain.Domain
 import com.cloud.domain.DomainVO
 import com.cloud.user.AccountService
 import com.cloud.user.DomainService
-import com.cloud.user.UserAccount
-import com.cloud.user.UserAccountVO
 import org.apache.cloudstack.api.command.LdapImportUsersCmd
 import org.apache.cloudstack.api.response.LdapUserResponse
 import org.apache.cloudstack.ldap.LdapManager
@@ -54,11 +52,11 @@ class LdapImportUsersCmdSpec extends spock.lang.Specification {
         ldapManager.getUsers() >> users
         LdapUserResponse response1 = new LdapUserResponse("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering")
         LdapUserResponse response2 = new LdapUserResponse("bob", "bob@test.com", "Robert", "Young", "cn=bob,ou=engineering,dc=cloudstack,dc=org", "engineering")
-	ldapManager.createLdapUserResponse(_) >>>[response1, response2]
+        ldapManager.createLdapUserResponse(_) >>> [response1, response2]
 
 
         Domain domain = new DomainVO("engineering", 1L, 1L, "engineering", UUID.randomUUID().toString())
-	domainService.getDomainByName("engineering", 1L) >>> [null, domain]
+        2 * domainService.getDomainByName("engineering", 1L) >>> [null, domain]
         1 * domainService.createDomain("engineering", 1L, "engineering", _) >> domain
 
         def ldapImportUsersCmd = new LdapImportUsersCmd(ldapManager, domainService, accountService)
@@ -82,11 +80,11 @@ class LdapImportUsersCmdSpec extends spock.lang.Specification {
         ldapManager.getUsersInGroup("TestGroup") >> users
         LdapUserResponse response1 = new LdapUserResponse("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering")
         LdapUserResponse response2 = new LdapUserResponse("bob", "bob@test.com", "Robert", "Young", "cn=bob,ou=engineering,dc=cloudstack,dc=org", "engineering")
-	ldapManager.createLdapUserResponse(_) >>>[response1, response2]
+        ldapManager.createLdapUserResponse(_) >>> [response1, response2]
 
 
         Domain domain = new DomainVO("TestGroup", 1L, 1L, "TestGroup", UUID.randomUUID().toString())
-	domainService.getDomainByName("TestGroup", 1L) >>> [null, domain]
+        1 * domainService.getDomainByName("TestGroup", 1L) >>> null
         1 * domainService.createDomain("TestGroup", 1L, "TestGroup", _) >> domain
 
         def ldapImportUsersCmd = new LdapImportUsersCmd(ldapManager, domainService, accountService)
@@ -111,17 +109,16 @@ class LdapImportUsersCmdSpec extends spock.lang.Specification {
         ldapManager.getUsersInGroup("TestGroup") >> users
         LdapUserResponse response1 = new LdapUserResponse("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering")
         LdapUserResponse response2 = new LdapUserResponse("bob", "bob@test.com", "Robert", "Young", "cn=bob,ou=engineering,dc=cloudstack,dc=org", "engineering")
-	ldapManager.createLdapUserResponse(_) >>>[response1, response2]
+        ldapManager.createLdapUserResponse(_) >>> [response1, response2]
 
 
         Domain domain = new DomainVO("TestDomain", 1L, 1L, "TestDomain", UUID.randomUUID().toString())
-	domainService.getDomainByName("TestDomain", 1L) >>> [null, domain]
+        1 * domainService.getDomain(1L) >> domain;
-	1 * domainService.createDomain("TestDomain", 1L, "TestDomain", _) >> domain
 
         def ldapImportUsersCmd = new LdapImportUsersCmd(ldapManager, domainService, accountService)
         ldapImportUsersCmd.accountType = 2;
         ldapImportUsersCmd.groupName = "TestGroup";
-	ldapImportUsersCmd.domainName = "TestDomain";
+        ldapImportUsersCmd.domainId = 1L;
 
         when: "LdapListUsersCmd is executed"
         ldapImportUsersCmd.execute()
@@ -141,16 +138,15 @@ class LdapImportUsersCmdSpec extends spock.lang.Specification {
         ldapManager.getUsers() >> users
         LdapUserResponse response1 = new LdapUserResponse("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering")
         LdapUserResponse response2 = new LdapUserResponse("bob", "bob@test.com", "Robert", "Young", "cn=bob,ou=engineering,dc=cloudstack,dc=org", "engineering")
-	ldapManager.createLdapUserResponse(_) >>>[response1, response2]
+        ldapManager.createLdapUserResponse(_) >>> [response1, response2]
 
 
         Domain domain = new DomainVO("TestDomain", 1L, 1L, "TestDomain", UUID.randomUUID().toString())
-	domainService.getDomainByName("TestDomain", 1L) >>> [null, domain]
+        1 * domainService.getDomain(1L) >> domain;
-	1 * domainService.createDomain("TestDomain", 1L, "TestDomain", _) >> domain
 
         def ldapImportUsersCmd = new LdapImportUsersCmd(ldapManager, domainService, accountService)
         ldapImportUsersCmd.accountType = 2;
-	ldapImportUsersCmd.domainName = "TestDomain";
+        ldapImportUsersCmd.domainId = 1L;
 
         when: "LdapListUsersCmd is executed"
         ldapImportUsersCmd.execute()
@@ -158,21 +154,29 @@ class LdapImportUsersCmdSpec extends spock.lang.Specification {
         ldapImportUsersCmd.responseObject.getResponses().size() == 2
     }
 
-    def "Test getDomain with no domain or group name specified specified"() {
+    def "Test getDomain"() {
         given: "We have an LdapManager, DomainService, two users and a LdapImportUsersCmd"
         def ldapManager = Mock(LdapManager)
         def domainService = Mock(DomainService)
         def accountService = Mock(AccountService)
         def ldapImportUsersCmd = new LdapImportUsersCmd(ldapManager, domainService, accountService)
-	ldapImportUsersCmd.domainName = varDomainName
+        ldapImportUsersCmd.domainId = varDomainId
         ldapImportUsersCmd.groupName = varGroupName
 
         def ldapUser1 = new LdapUser("rmurphy", "rmurphy@test.com", "Ryan", "Murphy", "cn=rmurphy,ou=engineering,dc=cloudstack,dc=org", "engineering")
         def ldapUser2 = new LdapUser("bob", "bob@test.com", "Robert", "Young", "cn=bob,ou=engineering,dc=cloudstack,dc=org", "engineering");
 
-	Domain domain = new DomainVO(expectedDomainName, 1L, 1L, expectedDomainName, UUID.randomUUID().toString())
+        Domain domain = new DomainVO(expectedDomainName, 1L, 1L, expectedDomainName, UUID.randomUUID().toString());
-	2 * domainService.getDomainByName(expectedDomainName, 1L) >>> [null, domain]
+        if (varDomainId != null) {
+            1 * domainService.getDomain(varDomainId) >> domain;
+        } else {
+            if(varGroupName != null) {
+                1 * domainService.getDomainByName(expectedDomainName, 1L) >> null
+            } else {
+                domainService.getDomainByName(expectedDomainName, 1L) >>> [null, domain]
+            }
             1 * domainService.createDomain(expectedDomainName, 1L, expectedDomainName, _) >> domain
+        }
 
         def result1 = ldapImportUsersCmd.getDomain(ldapUser1)
         def result2 = ldapImportUsersCmd.getDomain(ldapUser2)
@@ -180,11 +184,12 @@ class LdapImportUsersCmdSpec extends spock.lang.Specification {
         result1 == domain
         result2 == domain
         where: "The domain and group are set to the following values"
-	varDomainName | varGroupName | expectedDomainName
+        varDomainId | varGroupName | expectedDomainName
         null        | null         | "engineering"
-	"TestDomain" | null | "TestDomain"
+        1L          | null         | "TestDomain"
-	"TestDomain" | "TestGroup" | "TestDomain"
+        1L          | "TestGroup"  | "TestDomain"
         null        | "TestGroup"  | "TestGroup"
+        null        | "Test Group"  | "TestGroup"
 
     }
 

ui/dictionary.jsp

@@ -1219,6 +1219,7 @@ dictionary = {
 'label.zone.step.3.title': '<fmt:message key="label.zone.step.3.title" />',
 'label.zone.step.4.title': '<fmt:message key="label.zone.step.4.title" />',
 'label.zone.wide': '<fmt:message key="label.zone.wide" />',
+'label.ldap.group.name': '<fmt:message key="label.ldap.group.name" />',
 'managed.state': '<fmt:message key="managed.state" />',
 'message.acquire.new.ip': '<fmt:message key="message.acquire.new.ip" />',
 'message.acquire.new.ip.vpc': '<fmt:message key="message.acquire.new.ip.vpc" />',

ui/scripts/accountsWizard.js

@@ -161,14 +161,23 @@
                 validation: {
                     required: false
                 }
+            },
+            ldapGroupName: {
+                label: 'label.ldap.group.name',
+                docID: 'helpLdapGroupName',
+                validation: {
+                    required: false
                 }
+            }
+
         },
 
         action: function(args) {
             var array1 = [];
             var ldapStatus = isLdapEnabled();
-            console.log("creating user: " + args.username);
+            if (args.username) {
                 array1.push("&username=" + args.username);
+            }
 
             if (!ldapStatus) {
                 var password = args.data.password;
@@ -179,7 +188,7 @@
                 array1.push("&firstname=" + args.data.firstname);
                 array1.push("&lastname=" + args.data.lastname);
 
-                var password = args.data.password;
+                password = args.data.password;
                 if (md5Hashed) {
                     password = $.md5(password);
                 }
@@ -207,9 +216,27 @@
             if (args.data.networkdomain !== null && args.data.networkdomain.length > 0) {
                 array1.push("&networkdomain=" + args.data.networkdomain);
             }
+            if (args.groupname && args.groupname !== null && args.groupname.length > 0) {
+                array1.push("&group=" + args.groupname);
+            }
 
             if (ldapStatus) {
-                console.log("doing an ldap add");
+                if (args.groupname) {
+                    $.ajax({
+                        url: createURL('importLdapUsers' + array1.join("")),
+                        dataType: "json",
+                        async: false,
+                        success: function(json) {
+                            var count = json.ldapuserresponse.count;
+                            args.response.success({
+                                data: count
+                            });
+                        },
+                        error: function(XMLHttpResponse) {
+                            args.response.error(parseXMLHttpResponse(XMLHttpResponse));
+                        }
+                    });
+                } else if (args.username) {
                     $.ajax({
                         url: createURL('ldapCreateAccount' + array1.join("")),
                         dataType: "json",
@@ -224,8 +251,8 @@
                             args.response.error(parseXMLHttpResponse(XMLHttpResponse));
                         }
                     });
+                }
             } else {
-                console.log("doing normal user add");
                 $.ajax({
                     url: createURL('createAccount' + array1.join("")),
                     dataType: "json",

ui/scripts/docs.js

@@ -1196,5 +1196,9 @@ cloudStack.docs = {
     helpUploadVolumeChecksum: {
         desc: 'Use the hash that you created at the start of the volume upload procedure',
         externalLink: ''
+    },
+    helpLdapGroupName: {
+        desc: 'The group name from which you want to import LDAP users',
+        externalLink: ''
     }
 };

ui/scripts/ui-custom/accountsWizard.js

@@ -33,12 +33,27 @@
 
                 var completeAction = function() {
                     var data = cloudStack.serializeForm($form);
+                    var groupname = $.trim(data.ldapGroupName);
+                    if (groupname) {
+                        args.action({
+                            context: context,
+                            data: data,
+                            groupname: groupname,
+                            response: {
+                                error: function(message) {
+                                    if (message) {
+                                        cloudStack.dialog.notice({
+                                            message: message
+                                        });
+                                    }
+                                }
+                            }
+                        });
+                    } else {
                         var username = data.username;
                         var bulkAdd = (username instanceof Array);
                         if (bulkAdd) {
-                        console.log("doing bulk add");
                             for (var i = 0; i < username.length; i++) {
-                            console.log("creating user " + username[i]);
                                 args.action({
                                     context: context,
                                     data: data,
@@ -70,6 +85,7 @@
                                 }
                             });
                         }
+                    }
                 };
 
                 $wizard.click(function(event) {
@@ -92,6 +108,19 @@
 
                 if (ldapStatus) {
                     var $table = $wizard.find('.ldap-account-choice tbody');
+                    $("#label_ldap_group_name").live("keypress", function(event) {
+                        if ($table.find("#tr-groupname-message").length === 0) {
+                            $("<tr id='tr-groupname-message'>").appendTo($table).append("<td colspan=\"4\">All The users from the given group name will be imported</td>");
+                        }
+                        $table.find("tr").hide();
+                        $table.find("#tr-groupname-message").show();
+                    });
+                    $("#label_ldap_group_name").live("blur", function(event) {
+                        if (!$(this).val()) {
+                            $table.find("tr").show();
+                            $table.find("#tr-groupname-message").hide();
+                        }
+                    });
                     $.ajax({
                         url: createURL("listLdapUsers&listtype=new"),
                         dataType: "json",
@@ -100,7 +129,7 @@
                             if (json.ldapuserresponse.count > 0) {
                                 $(json.ldapuserresponse.LdapUser).each(function() {
                                     var result = $("<tr>");
-				    result.append("<td><input type=\"checkbox\" class=\"required\" name=\"username\" value=\"" + this.username + "\"></td>");
+                                    result.append("<td><input type=\"checkbox\" name=\"username\" value=\"" + this.username + "\"></td>");
                                     result.append("<td>" + this.firstname + " " + this.lastname + "</td>");
                                     result.append("<td>" + this.username + "</td>");
                                     result.append("<td>" + this.email + "</td>");
@@ -123,7 +152,6 @@
                         }
                     });
 
-                    //console.log(informationWithinLdap.$formContainer);
                     var informationWithinLdapForm = informationWithinLdap.$formContainer.find('form .form-item');
                     informationWithinLdapForm.find('.value #label_username').addClass('required');
                     informationWithinLdapForm.find('.value #password').addClass('required');
@@ -137,6 +165,10 @@
                     $wizard.removeClass('multi-wizard');
                 }
 
+                if (!ldapStatus) {
+                    delete args.informationNotInLdap.ldapGroupName;
+                }
+
                 var informationNotInLdap = cloudStack.dialog.createForm({
                     context: context,
                     noDialog: true,