You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2018/01/04 07:56:26 UTC

[GitHub] rhtyd closed pull request #2378: CLOUDSTACK-10205 make LinkDomainToLdap return UUID instead of internal id

rhtyd closed pull request #2378: CLOUDSTACK-10205 make LinkDomainToLdap return UUID instead of internal id
URL: https://github.com/apache/cloudstack/pull/2378
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/api/src/org/apache/cloudstack/api/ApiConstants.java b/api/src/org/apache/cloudstack/api/ApiConstants.java
index 55e8c285e6b..98b5ccb99a1 100644
--- a/api/src/org/apache/cloudstack/api/ApiConstants.java
+++ b/api/src/org/apache/cloudstack/api/ApiConstants.java
@@ -705,6 +705,7 @@
 
     public static final String HAS_ANNOTATION = "hasannotation";
     public static final String LAST_ANNOTATED = "lastannotated";
+    public static final String LDAP_DOMAIN = "ldapdomain";
 
     public enum HostDetails {
         all, capacity, events, stats, min;
diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/response/LinkDomainToLdapResponse.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/response/LinkDomainToLdapResponse.java
index b0032b04b4d..050eb6c3eb5 100644
--- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/response/LinkDomainToLdapResponse.java
+++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/response/LinkDomainToLdapResponse.java
@@ -27,12 +27,17 @@
 
     @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 @@
     @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() {
diff --git a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManagerImpl.java b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManagerImpl.java
index a4d340647f4..beb7a61cd70 100644
--- a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManagerImpl.java
+++ b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManagerImpl.java
@@ -44,6 +44,8 @@
 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 @@
     @Inject
     private LdapConfigurationDao _ldapConfigurationDao;
 
+    @Inject
+    private DomainDao domainDao;
+
     @Inject
     private LdapContextFactory _ldapContextFactory;
 
@@ -270,7 +275,14 @@ public LinkDomainToLdapResponse linkDomainToLdap(Long domainId, String type, Str
         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;
     }
 
diff --git a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LinkDomainToLdapCmdSpec.groovy b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LinkDomainToLdapCmdSpec.groovy
index 9d667bf4cfb..bad41e82805 100644
--- a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LinkDomainToLdapCmdSpec.groovy
+++ b/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


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services