HPCC-31761 LDAP caches first user's default file scope permission and…

… uses it for future requests Added a per user default file scope permission cache Signed-Off-By: Kenneth Rowland [email protected]
hpcc-systems · Jun 18, 2024 · c41e168 · c41e168
1 parent 1e1fedb
commit c41e168
Show file tree

Hide file tree

Showing 8 changed files with 78 additions and 8 deletions.
diff --git a/esp/applications/common/ldap/ldap.yaml b/esp/applications/common/ldap/ldap.yaml
@@ -21,4 +21,4 @@ ldap:
   ldapAdminVaultId: ""
   hpccAdminSecretKey: ""
   hpccAdminVaultId: ""
-  checkScopeScans: true
+  checkScopeScans: true
diff --git a/helm/hpcc/values.schema.json b/helm/hpcc/values.schema.json
@@ -1086,6 +1086,10 @@
         "checkScopeScans": {
           "type": "boolean",
           "description": "Only return iterated logical file metadata for files that user has scope permission to access"
+        },
+        "useLegacyDefaultFileScopePermissionCache": {
+          "type": "boolean",
+          "description": "Use legacy default filescope permissions that cached value for first user"
         }
       }
     },

diff --git a/initfiles/componentfiles/configxml/dali.xsl b/initfiles/componentfiles/configxml/dali.xsl
@@ -308,6 +308,9 @@
             <xsl:attribute name="adminGroupName">
                 <xsl:value-of select="/Environment/Software/LDAPServerProcess[@name=$ldapServerName]/@adminGroupName"/>
             </xsl:attribute>
+            <xsl:attribute name="useLegacyDefaultFileScopePermissionCache">
+                <xsl:value-of select="/Environment/Software/LDAPServerProcess[@name=$ldapServerName]/@useLegacyDefaultFileScopePermissionCache"/>
+            </xsl:attribute>
             <xsl:variable name="ldapServerNode" select="/Environment/Software/LDAPServerProcess[@name=$ldapServerName]"/>
             <xsl:if test="not($ldapServerNode)">
               <xsl:message terminate="yes">

diff --git a/initfiles/componentfiles/configxml/esp.xsl b/initfiles/componentfiles/configxml/esp.xsl
@@ -466,6 +466,9 @@
                         <xsl:otherwise/>
                     </xsl:choose>
                 </xsl:for-each>
+                <xsl:attribute name="useLegacyDefaultFileScopePermissionCache">
+                    <xsl:value-of select="/Environment/Software/LDAPServerProcess[@name=$ldapServer]/@useLegacyDefaultFileScopePermissionCache"/>
+                </xsl:attribute>
             </xsl:element>
         </xsl:for-each>
     </xsl:template>

diff --git a/system/security/LdapSecurity/ldapsecurity.cpp b/system/security/LdapSecurity/ldapsecurity.cpp
@@ -630,6 +630,8 @@ void CLdapSecManager::init(const char *serviceName, IPropertyTree* cfg)
     m_permissionsCache->setCacheTimeout( 60 * cacheTimeoutMinutes);
     m_permissionsCache->setTransactionalEnabled(true);
     m_permissionsCache->setSecManager(this);
+    m_useLegacyDefaultFileScopePermissionCaching = cfg->getPropBool("@useLegacyDefaultFileScopePermissionCache", m_useLegacyDefaultFileScopePermissionCaching);
+    m_permissionsCache->setUseLegacyDefaultFileScopePermissionCache(m_useLegacyDefaultFileScopePermissionCaching);
     m_passwordExpirationWarningDays = cfg->getPropInt(".//@passwordExpirationWarningDays", 10); //Default to 10 days
     m_checkViewPermissions = cfg->getPropBool(".//@checkViewPermissions", false);
     m_hpccInternalScope.set(queryDfsXmlBranchName(DXB_Internal)).append("::");//HpccInternal::

diff --git a/system/security/LdapSecurity/ldapsecurity.ipp b/system/security/LdapSecurity/ldapsecurity.ipp
@@ -323,6 +323,7 @@ private:
     static const SecFeatureSet s_safeFeatures = SMF_ALL_FEATURES;
     static const SecFeatureSet s_implementedFeatures = s_safeFeatures & ~(SMF_RetrieveUserData | SMF_RemoveResources);
     StringBuffer m_hpccInternalScope;
+    bool m_useLegacyDefaultFileScopePermissionCaching = true;
 
 public:
     IMPLEMENT_IINTERFACE

diff --git a/system/security/shared/caching.cpp b/system/security/shared/caching.cpp
@@ -549,7 +549,8 @@ inline void CPermissionsCache::removeAllManagedFileScopes()
 
     etc. Until full scope path checked, or no read permissions hit on ancestor scope.
 */
-static CriticalSection msCacheSyncCS;//for managed scopes cache syncronization
+static CriticalSection msCacheSyncCS;//for managed scopes cache synchronization
+static CriticalSection syncDefaultScopePermissions;//for cached default file scope permissions
 bool CPermissionsCache::queryPermsManagedFileScope(ISecUser& sec_user, const char * fullScope, StringBuffer& managedScope, SecAccessFlags * accessFlags)
 {
     unsigned start = msTick();
@@ -572,7 +573,15 @@ bool CPermissionsCache::queryPermsManagedFileScope(ISecUser& sec_user, const cha
             aindex_t count = m_secMgr->getManagedScopeTree(RT_FILE_SCOPE, nullptr, scopes);
             if (count)
                 addManagedFileScopes(scopes);
-            m_defaultPermission = SecAccess_Unknown;//trigger refresh
+            if (m_useLegacyDefaultFileScopePermissionCache)
+            {
+                m_defaultPermission = SecAccess_Unknown;
+            }
+            else
+            {
+                CriticalBlock defaultScopePermissionBlock(syncDefaultScopePermissions);
+                m_userDefaultFileScopePermissions.clear();
+            }
             time(&m_lastManagedFileScopesRefresh);
         }
     }
@@ -672,16 +681,47 @@ bool CPermissionsCache::queryPermsManagedFileScope(ISecUser& sec_user, const cha
 
 SecAccessFlags CPermissionsCache::queryDefaultPermission(ISecUser& user)
 {
-    if (m_defaultPermission == SecAccess_Unknown)
+    if (!m_secMgr)
+        return SecAccess_Full; // if no security manager, all full access to all scopes
+
+    if (m_useLegacyDefaultFileScopePermissionCache)
     {
-        if (m_secMgr)
+        if (m_defaultPermission == SecAccess_Unknown)
+        {
             m_defaultPermission = m_secMgr->queryDefaultPermission(user);
+            DBGLOG("Legacy default file scope permission set to %s(%d) for all users, based on User '%s'", getSecAccessFlagName(m_defaultPermission),
+                   m_defaultPermission, user.getName());
+        }
+        return m_defaultPermission;
+    }
+
+    SecAccessFlags defaultPermission = SecAccess_None;
+    const std::string username(user.getName());
+    bool addedToCache = false;
+    {
+        CriticalBlock defaultScopePermissionBlock(syncDefaultScopePermissions);
+        auto it = m_userDefaultFileScopePermissions.find(username);
+        if (it == m_userDefaultFileScopePermissions.end())
+        {
+            defaultPermission = m_secMgr->queryDefaultPermission(user);
+            m_userDefaultFileScopePermissions.emplace(username, defaultPermission);
+            addedToCache = true;
+        }
         else
-            m_defaultPermission = SecAccess_None;
+        {
+            defaultPermission = it->second;
+        }
+    }
+
+    if (addedToCache)
+    {
+        DBGLOG("Added user '%s' to default file scope permissions with access %s(%d)", username.c_str(), getSecAccessFlagName(defaultPermission),
+               defaultPermission);
     }
-    return m_defaultPermission;
 
+    return defaultPermission;
 }
+
 void CPermissionsCache::flush()
 {
     // MORE - is this safe? m_defaultPermossion and m_lastManagedFileScopesRefresh are unprotected,
@@ -702,8 +742,16 @@ void CPermissionsCache::flush()
             delete (*ui).second;
         m_userCache.clear();
     }
+    if (m_useLegacyDefaultFileScopePermissionCache)
+    {
+        m_defaultPermission = SecAccess_Unknown;
+    }
+    else
+    {
+        CriticalBlock defaultScopePermissionBlock(syncDefaultScopePermissions);
+        m_userDefaultFileScopePermissions.clear();
+    }
     m_lastManagedFileScopesRefresh = 0;
-    m_defaultPermission = SecAccess_Unknown;//trigger refresh
 }
 
 CPermissionsCache* CPermissionsCache::getInstance(const char * _secMgrClass)

diff --git a/system/security/shared/caching.hpp b/system/security/shared/caching.hpp
@@ -203,6 +203,12 @@ class CPermissionsCache : public CInterface
     bool queryPermsManagedFileScope(ISecUser& sec_user, const char * fullScope, StringBuffer& managedScope, SecAccessFlags * accessFlags);
     void setSecManager(ISecManager * secMgr) { m_secMgr = secMgr; }
     SecAccessFlags  queryDefaultPermission(ISecUser& user);
+    void setUseLegacyDefaultFileScopePermissionCache(bool useLegacy)
+    {
+        if (useLegacy)
+            DBGLOG("*** Setting default file scope permissions to use legacy mode which uses first retrieved permission for all users.");
+        m_useLegacyDefaultFileScopePermissionCache = useLegacy;
+    }
 private:
 
     typedef std::map<string, CResPermissionsCache*> MapResPermissionsCache;
@@ -221,11 +227,14 @@ class CPermissionsCache : public CInterface
     StringAttr                  m_secMgrClass;
 
     //Managed File Scope support
+    std::map<std::string, SecAccessFlags> m_userDefaultFileScopePermissions;
     SecAccessFlags              m_defaultPermission;
     map<string, ISecResource*>  m_managedFileScopesMap;
     mutable ReadWriteLock       m_scopesRWLock;//guards m_managedFileScopesMap
     ISecManager *               m_secMgr;
     time_t                      m_lastManagedFileScopesRefresh;
+
+    bool m_useLegacyDefaultFileScopePermissionCache = false;
 };
 
 time_t getThreadCreateTime();