HPCC-21367 Refactor LDAP security manager getResourcesEx call

When querying LDAP for resources that are stored in a hierarchical manner, ie file scopes
and workunit scopes, there is no need to make recursive calls to LDAP. Instead,
perform the canonical search as implemented in getManagedFileScopes to query the
entire subtree in a single call. This PR refactors the getManagedFileScopes to
search either workunit or file scopes, and modifies getResources/getResourceEx
to call it instead of recursing

Signed-off-by: Russ Whitehead <william.whitehead@lexisnexisrisk.com>

system/security/LdapSecurity/ldapconnection.cpp

@@ -3457,14 +3457,20 @@ public:
 
     virtual bool getResources(LDAP* ld, SecResourceType rtype, const char * basedn, const char* prefix, IArrayOf<ISecResource>& resources)
     {
+        if(rtype == RT_FILE_SCOPE || rtype == RT_WORKUNIT_SCOPE)
+        {
+            assertex(isEmptyString(prefix));
+
+            getManagedScopeTree(rtype, basedn, resources);
+            return true;
+        }
+
         char        *attribute;
         LDAPMessage *message;
 
         StringBuffer basednbuf;
         LdapUtils::normalizeDn(basedn, m_ldapconfig->getBasedn(), basednbuf);
-        StringBuffer filter("objectClass=*");
 
-        TIMEVAL timeOut = {m_ldapconfig->getLdapTimeout(),0};
         const char* fldname;
         LdapServerType servertype = m_ldapconfig->getServerType();
         if(servertype == ACTIVE_DIRECTORY && (rtype == RT_DEFAULT || rtype == RT_MODULE || rtype == RT_SERVICE))
@@ -3473,7 +3479,7 @@ public:
             fldname = "ou";
         char        *attrs[] = {(char*)fldname, "description", NULL};
 
-        CPagedLDAPSearch pagedSrch(ld, m_ldapconfig->getLdapTimeout(), (char*)basednbuf.str(), LDAP_SCOPE_ONELEVEL, (char*)filter.str(), attrs);
+        CPagedLDAPSearch pagedSrch(ld, m_ldapconfig->getLdapTimeout(), (char*)basednbuf.str(), LDAP_SCOPE_ONELEVEL, "objectClass=*", attrs);
         for (message = pagedSrch.getFirstEntry(); message; message = pagedSrch.getNextEntry())
         {
             // Go through the search results by checking message types
@@ -3511,16 +3517,6 @@ public:
                 CLdapSecResource* resource = new CLdapSecResource(resourcename.str());
                 resource->setDescription(descbuf.str());
                 resources.append(*resource);
-                if(rtype == RT_FILE_SCOPE || rtype == RT_WORKUNIT_SCOPE)
-                {
-                    StringBuffer nextbasedn;
-                    nextbasedn.append("ou=").append(curname.str()).append(",").append(basedn);
-                    StringBuffer nextprefix;
-                    if(prefix != NULL && *prefix != '\0')
-                        nextprefix.append(prefix);
-                    nextprefix.append(curname.str()).append("::");
-                    getResources(ld, rtype, nextbasedn.str(), nextprefix.str(), resources);
-                }
             }
         }
 
@@ -3529,20 +3525,28 @@ public:
 
     virtual bool getResourcesEx(SecResourceType rtype, const char * basedn, const char* prefix, const char* searchstr, IArrayOf<ISecResource>& resources)
     {
+        if(rtype == RT_FILE_SCOPE || rtype == RT_WORKUNIT_SCOPE)
+        {
+            assertex(isEmptyString(searchstr));
+            assertex(isEmptyString(prefix));
+
+            getManagedScopeTree(rtype, basedn, resources);
+            return true;
+        }
+
         char        *attribute;
         LDAPMessage *message;
 
         StringBuffer basednbuf;
         LdapUtils::normalizeDn(basedn, m_ldapconfig->getBasedn(), basednbuf);
         StringBuffer filter("objectClass=*");
+
         if(searchstr && *searchstr && strcmp(searchstr, "*") != 0)
         {
             filter.insert(0, "(&(");
             filter.appendf(")(|(%s=*%s*)))", "uNCName", searchstr);
         }
 
-        TIMEVAL timeOut = {m_ldapconfig->getLdapTimeout(),0};
-
         Owned<ILdapConnection> lconn = m_connections->getConnection();
         LDAP* ld = ((CLdapConnection*)lconn.get())->getLd();
 
@@ -3582,6 +3586,7 @@ public:
                     }
                 }
             }
+
             if(curname.length() == 0)
                 continue;
             StringBuffer resourcename;
@@ -3591,16 +3596,6 @@ public:
             CLdapSecResource* resource = new CLdapSecResource(resourcename.str());
             resource->setDescription(descbuf.str());
             resources.append(*resource);
-            if(rtype == RT_FILE_SCOPE || rtype == RT_WORKUNIT_SCOPE)
-            {
-                StringBuffer nextbasedn;
-                nextbasedn.append("ou=").append(curname.str()).append(",").append(basedn);
-                StringBuffer nextprefix;
-                if(prefix != NULL && *prefix != '\0')
-                    nextprefix.append(prefix);
-                nextprefix.append(curname.str()).append("::");
-                getResources(ld, rtype, nextbasedn.str(), nextprefix.str(), resources);
-            }
         }
 
         return true;
@@ -6173,20 +6168,20 @@ private:
         return addResource(RT_FILE_SCOPE, user, resource, PT_ADMINISTRATORS_AND_USER, m_ldapconfig->getResourceBasedn(RT_FILE_SCOPE));
     }
 
-    virtual aindex_t getManagedFileScopes(IArrayOf<ISecResource>& scopes)
+    virtual aindex_t getManagedScopeTree(SecResourceType rtype, const char * basedn, IArrayOf<ISecResource>& scopes)
     {
         //Get array of all file scopes listed in files baseDN
         StringBuffer basednbuf;
-        LdapUtils::normalizeDn(m_ldapconfig->getResourceBasedn(RT_FILE_SCOPE), m_ldapconfig->getBasedn(), basednbuf);
+        LdapUtils::normalizeDn(basedn ? basedn : m_ldapconfig->getResourceBasedn(rtype), m_ldapconfig->getBasedn(), basednbuf);
         basednbuf.toLowerCase();//Will look something like "ou=files,ou=dataland_ecl,dc=internal,dc=sds". Lowercase ensures proper strstr with StringArray elements below
 
-        TIMEVAL timeOut = {m_ldapconfig->getLdapTimeout(),0};
         Owned<ILdapConnection> lconn = m_connections->getConnection();
         LDAP* ld = ((CLdapConnection*)lconn.get())->getLd();
         char *attrs[] = {"canonicalName", NULL};
 
         //Call LDAP to get the complete OU tree underneath basdnbuf
         CPagedLDAPSearch pagedSrch(ld, m_ldapconfig->getLdapTimeout(), (char*)basednbuf.str(), LDAP_SCOPE_SUBTREE, "objectClass=*", attrs);
+        StringArray arrScopes;
         for (LDAPMessage *message = pagedSrch.getFirstEntry(); message; message = pagedSrch.getNextEntry())
         {
             CLDAPGetAttributesWrapper   atts(ld, message);
@@ -6224,14 +6219,26 @@ private:
 
                         if (!sb.isEmpty())
                         {
-                            CLdapSecResource* resource = new CLdapSecResource(sb);
-                            scopes.append(*resource);
+                            arrScopes.append(sb);
                         }
                     }
                 }
             }
         }
-        DBGLOG("getManagedFileScopes() found %d scopes under '%s'", scopes.length(), basednbuf.str());
+
+        //Build sorted IArrayOf<ISecResource> from arrScopes
+        if (arrScopes.length())
+        {
+            arrScopes.sortAscii(false);
+            ForEachItemIn(i, arrScopes)
+            {
+                const char * scope= arrScopes.item(i);
+                CLdapSecResource* resource = new CLdapSecResource(scope);
+                scopes.append(*resource);
+            }
+        }
+
+        DBGLOG("getManagedScopeTree() found %d scopes under '%s'", scopes.length(), basednbuf.str());
         return scopes.length();
     }
 

system/security/LdapSecurity/ldapconnection.hpp

@@ -312,7 +312,7 @@ interface ILdapClient : extends IInterface
     virtual ILdapConfig* queryConfig() = 0;
     virtual const char* getPasswordStorageScheme() = 0;
     virtual bool createUserScope(ISecUser& user) = 0;
-    virtual aindex_t getManagedFileScopes(IArrayOf<ISecResource>& scopes) = 0;
+    virtual aindex_t getManagedScopeTree(SecResourceType rtype, const char * basedn, IArrayOf<ISecResource>& scopes) = 0;
     virtual SecAccessFlags queryDefaultPermission(ISecUser& user) = 0;
 
     //Data View related interfaces

system/security/LdapSecurity/ldapsecurity.cpp

@@ -1468,9 +1468,9 @@ bool CLdapSecManager::createUserScopes()
 }
 
 
-aindex_t CLdapSecManager::getManagedFileScopes(IArrayOf<ISecResource>& scopes)
+aindex_t CLdapSecManager::getManagedScopeTree(SecResourceType rtype, const char * basedn, IArrayOf<ISecResource>& scopes)
 {
-    return m_ldap_client->getManagedFileScopes(scopes);
+    return m_ldap_client->getManagedScopeTree(rtype, basedn, scopes);
 }
 
 SecAccessFlags CLdapSecManager::queryDefaultPermission(ISecUser& user)

system/security/LdapSecurity/ldapsecurity.ipp

@@ -469,7 +469,7 @@ public:
     }
 
     virtual bool createUserScopes();
-    virtual aindex_t getManagedFileScopes(IArrayOf<ISecResource>& scopes);
+    virtual aindex_t getManagedScopeTree(SecResourceType rtype, const char * basedn, IArrayOf<ISecResource>& scopes);
     virtual SecAccessFlags queryDefaultPermission(ISecUser& user);
     virtual bool clearPermissionsCache(ISecUser &user);
     virtual bool authenticateUser(ISecUser & user, bool * superUser);

system/security/shared/basesecurity.hpp

@@ -240,7 +240,7 @@ public:
         throwUnexpected();
     }
 
-    aindex_t getManagedFileScopes(IArrayOf<ISecResource>& scopes)
+    aindex_t getManagedScopeTree(SecResourceType rtype, const char * basedn, IArrayOf<ISecResource>& scopes)
     {
         throwUnexpected();
     }

system/security/shared/caching.cpp

@@ -567,7 +567,7 @@ bool CPermissionsCache::queryPermsManagedFileScope(ISecUser& sec_user, const cha
         {
             removeAllManagedFileScopes();
             IArrayOf<ISecResource> scopes;
-            aindex_t count = m_secMgr->getManagedFileScopes(scopes);
+            aindex_t count = m_secMgr->getManagedScopeTree(RT_FILE_SCOPE, nullptr, scopes);
             if (count)
                 addManagedFileScopes(scopes);
             m_defaultPermission = SecAccess_Unknown;//trigger refresh

system/security/shared/seclib.hpp

@@ -339,7 +339,7 @@ interface ISecManager : extends IInterface
     virtual const char * getDescription() = 0;
     virtual unsigned getPasswordExpirationWarningDays() = 0;
     virtual bool createUserScopes() = 0;
-    virtual aindex_t getManagedFileScopes(IArrayOf<ISecResource>& scopes) = 0;
+    virtual aindex_t getManagedScopeTree(SecResourceType rtype, const char * basedn, IArrayOf<ISecResource>& scopes) = 0;
     virtual SecAccessFlags queryDefaultPermission(ISecUser& user) = 0;
     virtual bool clearPermissionsCache(ISecUser & user) = 0;
     virtual bool authenticateUser(ISecUser & user, bool * superUser) = 0;