From 40cf38ad5ec624c4b0139c169ed2591be8924944 Mon Sep 17 00:00:00 2001 From: Ken Rowland Date: Tue, 14 May 2024 13:36:29 -0400 Subject: [PATCH] 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 kenneth.rowland@lexisnexisrisk.com --- esp/applications/common/ldap/ldap.yaml | 1 + helm/hpcc/values.schema.json | 4 ++ initfiles/componentfiles/configxml/dali.xsl | 3 + initfiles/componentfiles/configxml/esp.xsl | 3 + system/security/LdapSecurity/ldapsecurity.cpp | 1 + system/security/shared/caching.cpp | 56 ++++++++++++++++--- system/security/shared/caching.hpp | 9 +++ 7 files changed, 69 insertions(+), 8 deletions(-) diff --git a/esp/applications/common/ldap/ldap.yaml b/esp/applications/common/ldap/ldap.yaml index 3f87d56f518..b5077d30347 100644 --- a/esp/applications/common/ldap/ldap.yaml +++ b/esp/applications/common/ldap/ldap.yaml @@ -22,3 +22,4 @@ ldap: hpccAdminSecretKey: "" hpccAdminVaultId: "" checkScopeScans: true + useLegacyDefaultFileScopePermissionCache: false diff --git a/helm/hpcc/values.schema.json b/helm/hpcc/values.schema.json index b42f1f297d3..fcea15c7b2a 100644 --- 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 index ffafa3199cc..5983537297d 100644 --- a/initfiles/componentfiles/configxml/dali.xsl +++ b/initfiles/componentfiles/configxml/dali.xsl @@ -308,6 +308,9 @@ + + + diff --git a/initfiles/componentfiles/configxml/esp.xsl b/initfiles/componentfiles/configxml/esp.xsl index c567856f282..8fca1a45b39 100644 --- a/initfiles/componentfiles/configxml/esp.xsl +++ b/initfiles/componentfiles/configxml/esp.xsl @@ -466,6 +466,9 @@ + + + diff --git a/system/security/LdapSecurity/ldapsecurity.cpp b/system/security/LdapSecurity/ldapsecurity.cpp index 7bd62e4e3d0..3cc400c2f54 100644 --- a/system/security/LdapSecurity/ldapsecurity.cpp +++ b/system/security/LdapSecurity/ldapsecurity.cpp @@ -630,6 +630,7 @@ void CLdapSecManager::init(const char *serviceName, IPropertyTree* cfg) m_permissionsCache->setCacheTimeout( 60 * cacheTimeoutMinutes); m_permissionsCache->setTransactionalEnabled(true); m_permissionsCache->setSecManager(this); + m_permissionsCache->setUseLegacyDefaultFileScopePermissionCache(cfg->getPropBool("@useLegacyDefaultFileScopePermissionCache", false)); 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/shared/caching.cpp b/system/security/shared/caching.cpp index 27a7fdb6707..5a6fbdc5a79 100644 --- 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,39 @@ 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); - else - m_defaultPermission = SecAccess_None; + 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; } - return m_defaultPermission; + SecAccessFlags defaultPermission = SecAccess_None; + CriticalBlock defaultScopePermissionBlock(syncDefaultScopePermissions); + const std::string username(user.getName()); + auto it = m_userDefaultFileScopePermissions.find(username); + if (it == m_userDefaultFileScopePermissions.end()) + { + defaultPermission = m_secMgr->queryDefaultPermission(user); + m_userDefaultFileScopePermissions.emplace(username, defaultPermission); + DBGLOG("Added user '%s' to default file scope permissions with access %s(%d)", username.c_str(), getSecAccessFlagName(defaultPermission), + defaultPermission); + } + else + { + defaultPermission = it->second; + } + + return defaultPermission; } + void CPermissionsCache::flush() { // MORE - is this safe? m_defaultPermossion and m_lastManagedFileScopesRefresh are unprotected, @@ -702,8 +734,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 index ac37cc9f457..243b71ef486 100644 --- 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 MapResPermissionsCache; @@ -221,11 +227,14 @@ class CPermissionsCache : public CInterface StringAttr m_secMgrClass; //Managed File Scope support + std::map m_userDefaultFileScopePermissions; SecAccessFlags m_defaultPermission; map m_managedFileScopesMap; mutable ReadWriteLock m_scopesRWLock;//guards m_managedFileScopesMap ISecManager * m_secMgr; time_t m_lastManagedFileScopesRefresh; + + bool m_useLegacyDefaultFileScopePermissionCache = false; }; time_t getThreadCreateTime();