CLOUDSTACK-10205: LinkDomainToLdap returns UUID instead of internal id (#2378)

The internal id is not usefull to the user. It is a bug to return it instead of a uuid.
In the process of fixing the above "name" was deprecated in favour of "ldap_domain".

api/src/org/apache/cloudstack/api/ApiConstants.java

@@ -707,6 +707,7 @@ public class ApiConstants {
 
     public static final String HAS_ANNOTATION = "hasannotation";
     public static final String LAST_ANNOTATED = "lastannotated";
+    public static final String LDAP_DOMAIN = "ldapdomain";
 
 
     public enum HostDetails {

plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/response/LinkDomainToLdapResponse.java

@@ -27,12 +27,17 @@ public class LinkDomainToLdapResponse extends BaseResponse {
 
     @SerializedName(ApiConstants.DOMAIN_ID)
     @Param(description = "id of the Domain which is linked to LDAP")
-    private long domainId;
+    private String domainId;
 
+    @Deprecated
     @SerializedName(ApiConstants.NAME)
     @Param(description = "name of the group or OU in LDAP which is linked to the domain")
     private String name;
 
+    @SerializedName(ApiConstants.LDAP_DOMAIN)
+    @Param(description = "name of the group or OU in LDAP which is linked to the domain")
+    private String ldapDomain;
+
     @SerializedName(ApiConstants.TYPE)
     @Param(description = "type of the name in LDAP which is linke to the domain")
     private String type;
@@ -45,19 +50,25 @@ public class LinkDomainToLdapResponse extends BaseResponse {
     @Param(description = "Domain Admin accountId that is created")
     private String adminId;
 
-    public LinkDomainToLdapResponse(long domainId, String type, String name, short accountType) {
+    public LinkDomainToLdapResponse(String domainId, String type, String ldapDomain, short accountType) {
         this.domainId = domainId;
-        this.name = name;
+        this.name = ldapDomain;
+        this.ldapDomain = ldapDomain;
         this.type = type;
         this.accountType = accountType;
     }
 
-    public long getDomainId() {
+    public String getDomainId() {
         return domainId;
     }
 
+    public String getLdapDomain() {
+        return ldapDomain == null ? name : ldapDomain;
+    }
+
+    @Deprecated
     public String getName() {
-        return name;
+        return ldapDomain == null ? name : ldapDomain;
     }
 
     public String getType() {

plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManagerImpl.java

@@ -44,6 +44,8 @@ import org.apache.commons.lang.Validate;
 import org.apache.log4j.Logger;
 import org.springframework.stereotype.Component;
 
+import com.cloud.domain.DomainVO;
+import com.cloud.domain.dao.DomainDao;
 import com.cloud.exception.InvalidParameterValueException;
 import com.cloud.utils.Pair;
 
@@ -54,6 +56,9 @@ public class LdapManagerImpl implements LdapManager, LdapValidator {
     @Inject
     private LdapConfigurationDao _ldapConfigurationDao;
 
+    @Inject
+    private DomainDao domainDao;
+
     @Inject
     private LdapContextFactory _ldapContextFactory;
 
@@ -270,7 +275,14 @@ public class LdapManagerImpl implements LdapManager, LdapValidator {
         Validate.isTrue(accountType==0 || accountType==2, "accountype should be either 0(normal user) or 2(domain admin)");
         LinkType linkType = LdapManager.LinkType.valueOf(type.toUpperCase());
         LdapTrustMapVO vo = _ldapTrustMapDao.persist(new LdapTrustMapVO(domainId, linkType, name, accountType));
-        LinkDomainToLdapResponse response = new LinkDomainToLdapResponse(vo.getDomainId(), vo.getType().toString(), vo.getName(), vo.getAccountType());
+        DomainVO domain = domainDao.findById(vo.getDomainId());
+        String domainUuid = "<unknown>";
+        if (domain == null) {
+            s_logger.error("no domain in database for id " + vo.getDomainId());
+        } else {
+            domainUuid = domain.getUuid();
+        }
+        LinkDomainToLdapResponse response = new LinkDomainToLdapResponse(domainUuid, vo.getType().toString(), vo.getName(), vo.getAccountType());
         return response;
     }
 

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

@@ -60,7 +60,7 @@ class LinkDomainToLdapCmdSpec extends Specification {
             thrown(ServerApiException)
     }
     def "test valid params without admin"(){
-        LinkDomainToLdapResponse response = new LinkDomainToLdapResponse(1, "GROUP", "CN=test,DC=ccp,DC=citrix,DC=com", (short)2)
+        LinkDomainToLdapResponse response = new LinkDomainToLdapResponse("1", "GROUP", "CN=test,DC=ccp,DC=citrix,DC=com", (short)2)
         _ldapManager.linkDomainToLdap(_,_,_,_) >> response
         when:
             linkDomainToLdapCmd.execute()
@@ -71,7 +71,7 @@ class LinkDomainToLdapCmdSpec extends Specification {
     }
 
     def "test with valid params and with disabled admin"() {
-        def domainId = 1;
+        def domainId = "1";
         def type = "GROUP";
         def name = "CN=test,DC=ccp,DC=Citrix,DC=com"
         def accountType = 2;
@@ -99,13 +99,13 @@ class LinkDomainToLdapCmdSpec extends Specification {
     }
 
     def "test with valid params and with admin who exist in cloudstack already"() {
-        def domainId = 1;
+        def domainId = 1L;
         def type = "GROUP";
         def name = "CN=test,DC=ccp,DC=Citrix,DC=com"
         def accountType = 2;
         def username = "admin"
 
-        LinkDomainToLdapResponse response = new LinkDomainToLdapResponse(domainId, type, name, (short)accountType)
+        LinkDomainToLdapResponse response = new LinkDomainToLdapResponse(domainId.toString(), type, name, (short)accountType)
         _ldapManager.linkDomainToLdap(_,_,_,_) >> response
         _ldapManager.getUser(username, type, name) >> new LdapUser(username, "admin@ccp.citrix.com", "Admin", "Admin", name, "ccp", false)
 
@@ -122,21 +122,21 @@ class LinkDomainToLdapCmdSpec extends Specification {
         LinkDomainToLdapResponse result = (LinkDomainToLdapResponse)linkDomainToLdapCmd.getResponseObject()
         result.getObjectName() == "LinkDomainToLdap"
         result.getResponseName() == linkDomainToLdapCmd.getCommandName()
-        result.getDomainId() == domainId
+        result.getDomainId() == domainId.toString()
         result.getType() == type
         result.getName() == name
         result.getAdminId() == null
     }
 
     def "test with valid params and with admin who doesnt exist in cloudstack"() {
-        def domainId = 1;
+        def domainId = 1L;
         def type = "GROUP";
         def name = "CN=test,DC=ccp,DC=Citrix,DC=com"
         def accountType = 2;
         def username = "admin"
         def accountId = 24
 
-        LinkDomainToLdapResponse response = new LinkDomainToLdapResponse(domainId, type, name, (short)accountType)
+        LinkDomainToLdapResponse response = new LinkDomainToLdapResponse(domainId.toString(), type, name, (short)accountType)
         _ldapManager.linkDomainToLdap(_,_,_,_) >> response
         _ldapManager.getUser(username, type, name) >> new LdapUser(username, "admin@ccp.citrix.com", "Admin", "Admin", name, "ccp", false)
 
@@ -157,20 +157,20 @@ class LinkDomainToLdapCmdSpec extends Specification {
         LinkDomainToLdapResponse result = (LinkDomainToLdapResponse)linkDomainToLdapCmd.getResponseObject()
         result.getObjectName() == "LinkDomainToLdap"
         result.getResponseName() == linkDomainToLdapCmd.getCommandName()
-        result.getDomainId() == domainId
+        result.getDomainId() == domainId.toString()
         result.getType() == type
         result.getName() == name
         result.getAdminId() == String.valueOf(accountId)
     }
 
     def "test when admin doesnt exist in ldap"() {
-        def domainId = 1;
+        def domainId = 1L;
         def type = "GROUP";
         def name = "CN=test,DC=ccp,DC=Citrix,DC=com"
         def accountType = 2;
         def username = "admin"
 
-        LinkDomainToLdapResponse response = new LinkDomainToLdapResponse(domainId, type, name, (short)accountType)
+        LinkDomainToLdapResponse response = new LinkDomainToLdapResponse(domainId.toString(), type, name, (short)accountType)
         _ldapManager.linkDomainToLdap(_,_,_,_) >> response
         _ldapManager.getUser(username, type, name) >> {throw new NoLdapUserMatchingQueryException("get ldap user failed from mock")}
 
@@ -185,7 +185,7 @@ class LinkDomainToLdapCmdSpec extends Specification {
         LinkDomainToLdapResponse result = (LinkDomainToLdapResponse)linkDomainToLdapCmd.getResponseObject()
         result.getObjectName() == "LinkDomainToLdap"
         result.getResponseName() == linkDomainToLdapCmd.getCommandName()
-        result.getDomainId() == domainId
+        result.getDomainId() == domainId.toString()
         result.getType() == type
         result.getName() == name
         result.getAdminId() == null
@@ -195,14 +195,14 @@ class LinkDomainToLdapCmdSpec extends Specification {
      * api should not fail in this case as link domain to ldap is successful
      */
     def "test when create user account throws a run time exception"() {
-        def domainId = 1;
+        def domainId = 1L;
         def type = "GROUP";
         def name = "CN=test,DC=ccp,DC=Citrix,DC=com"
         def accountType = 2;
         def username = "admin"
         def accountId = 24
 
-        LinkDomainToLdapResponse response = new LinkDomainToLdapResponse(domainId, type, name, (short)accountType)
+        LinkDomainToLdapResponse response = new LinkDomainToLdapResponse(domainId.toString(), type, name, (short)accountType)
         _ldapManager.linkDomainToLdap(_,_,_,_) >> response
         _ldapManager.getUser(username, type, name) >> new LdapUser(username, "admin@ccp.citrix.com", "Admin", "Admin", name, "ccp", false)
 
@@ -223,7 +223,7 @@ class LinkDomainToLdapCmdSpec extends Specification {
         LinkDomainToLdapResponse result = (LinkDomainToLdapResponse)linkDomainToLdapCmd.getResponseObject()
         result.getObjectName() == "LinkDomainToLdap"
         result.getResponseName() == linkDomainToLdapCmd.getCommandName()
-        result.getDomainId() == domainId
+        result.getDomainId() == domainId.toString()
         result.getType() == type
         result.getName() == name
         result.getAdminId() == null