Merge pull request #11556 from RussWhitehead/DaliAuth

HPCC-20284 Dali calls to get scope permissions can fail

Reviewed-by: Gavin Halliday <ghalliday@hpccsystems.com>

dali/server/daldap.cpp

@@ -146,8 +146,6 @@ public:
         Owned<ISecUser> user = ldapsecurity->createUser(username);
         user->credentials().setPassword(password);
 
-        bool authenticated = false;
-
         //Check that the digital signature provided by the caller (signature of
         //caller's "scope;username;timeStamp") matches what we expect it to be
         if (!isEmptyString(reqSignature))
@@ -186,13 +184,15 @@ public:
                     return SecAccess_None;//deny
                 }
 
-                authenticated = true;//Digital signature verified
+                //Mark user as authenticated. The call below to authenticateUser
+                //will add this user to the LDAP cache
+                user->setAuthenticateStatus(AS_AUTHENTICATED);
             }
             else
                 ERRLOG("LDAP: getPermissions(%s) scope=%s user=%s digital signature support not available",key?key:"NULL",obj?obj:"NULL",username.str());
         }
 
-        if (!authenticated && !ldapsecurity->authenticateUser(*user, NULL))
+        if (!ldapsecurity->authenticateUser(*user, NULL))
         {
             ERRLOG("LDAP: getPermissions(%s) scope=%s user=%s fails LDAP authentication",key?key:"NULL",obj?obj:"NULL",username.str());
             return SecAccess_None;//deny

system/security/LdapSecurity/ldapsecurity.cpp

@@ -647,8 +647,6 @@ bool CLdapSecManager::authenticate(ISecUser* user)
         return false;
     }
 
-    user->setAuthenticateStatus(AS_UNKNOWN);
-
     bool isCaching = m_permissionsCache->isCacheEnabled() && !m_usercache_off;//caching enabled?
     bool isUserCached = false;
     Owned<ISecUser> cachedUser = new CLdapSecUser(user->getName(), "");
@@ -658,6 +656,13 @@ bool CLdapSecManager::authenticate(ISecUser* user)
         isUserCached = m_permissionsCache->lookup(*cachedUser);//populate cachedUser with cached values
     }
 
+    if (AS_AUTHENTICATED == user->getAuthenticateStatus())
+    {
+        if(isCaching && !isUserCached)
+            m_permissionsCache->add(*user);
+        return true;
+    }
+
     //Verify provided signature if present
     IDigitalSignatureManager * pDSM = queryDigitalSignatureManagerInstanceFromEnv();
     if (pDSM && pDSM->isDigiVerifierConfigured() && !isEmptyString(user->credentials().getSignature()))
@@ -694,16 +699,19 @@ bool CLdapSecManager::authenticate(ISecUser* user)
 
     if (AS_AUTHENTICATED == user->getAuthenticateStatus())
     {
-        if (pDSM && pDSM->isDigiSignerConfigured() && isEmptyString(user->credentials().getSignature()))
-        {
-            //Set user digital signature
-            StringBuffer b64Signature;
-            pDSM->digiSign(user->getName(), b64Signature);
-            user->credentials().setSignature(b64Signature);
-        }
-
         if (isCaching)
             m_permissionsCache->add(*user);
+        else if (isEmptyString(user->credentials().getPassword()) && (0 == user->credentials().getSessionToken()) && isEmptyString(user->credentials().getSignature()))
+        {
+            //No need to sign if password or authenticated session based user
+            if (pDSM && pDSM->isDigiSignerConfigured())
+            {
+               //Set user digital signature
+               StringBuffer b64Signature;
+               pDSM->digiSign(user->getName(), b64Signature);
+               user->credentials().setSignature(b64Signature);
+            }
+        }
     }
 
     return AS_AUTHENTICATED == user->getAuthenticateStatus();

system/security/plugins/htpasswdSecurity/CMakeLists.txt

@@ -28,7 +28,6 @@ HPCC_ADD_SUBDIRECTORY (initfiles)
 
 set (    SRCS
          ${HPCC_SOURCE_DIR}/system/security/shared/authmap.cpp
-         ${HPCC_SOURCE_DIR}/system/security/shared/caching.cpp
          ${CMAKE_CURRENT_SOURCE_DIR}/htpasswdSecurity.cpp
     )
 

system/security/plugins/singleuserSecurity/CMakeLists.txt

@@ -28,7 +28,6 @@ HPCC_ADD_SUBDIRECTORY (initfiles)
 
 set (    SRCS
          ${HPCC_SOURCE_DIR}/system/security/shared/authmap.cpp
-         ${HPCC_SOURCE_DIR}/system/security/shared/caching.cpp
          ${CMAKE_CURRENT_SOURCE_DIR}/singleUserSecurity.cpp
     )
 

system/security/shared/caching.cpp

@@ -17,6 +17,7 @@
 
 #include "caching.hpp"
 #include "jtime.hpp"
+#include "digisign.hpp"
 
 //define a container for multiple instances of a security manager cache
 typedef map<string, CPermissionsCache*> MapCache;
@@ -358,7 +359,7 @@ bool CPermissionsCache::lookup(ISecUser& sec_user)
             const char* cachedpw = user->queryUser()->credentials().getPassword();
             const char * pw = sec_user.credentials().getPassword();
 
-            if ((sec_user.credentials().getSessionToken() != 0) ||  !isEmptyString(sec_user.credentials().getSignature()))
+            if ((sec_user.credentials().getSessionToken() != 0) || !isEmptyString(sec_user.credentials().getSignature()) || !isEmptyString(user->queryUser()->credentials().getSignature()) )
             {//presence of session token or signature means user is authenticated
 #ifdef _DEBUG
                 DBGLOG("CACHE: CPermissionsCache Found validated user %s", username);
@@ -437,6 +438,18 @@ void CPermissionsCache::add(ISecUser& sec_user)
 #ifdef _DEBUG
     DBGLOG("CACHE: CPermissionsCache Adding cached user %s", username);
 #endif
+    if (isEmptyString(sec_user.credentials().getPassword()) && (0 == sec_user.credentials().getSessionToken()) && isEmptyString(sec_user.credentials().getSignature()))
+    {
+        //No need to sign if password or authenticated session based user
+        IDigitalSignatureManager * pDSM = queryDigitalSignatureManagerInstanceFromEnv();
+        if (pDSM && pDSM->isDigiSignerConfigured())
+        {
+            //Set user digital signature
+            StringBuffer b64Signature;
+            pDSM->digiSign(sec_user.getName(), b64Signature);
+            sec_user.credentials().setSignature(b64Signature);//callers sec_user will now contain signature
+        }
+    }
     m_userCache[username] = new CachedUser(LINK(&sec_user));
 }