Merge branch 'pu/pm/LdapAdSyncGroupsFixes' into '2024.11'

tweak(TB Ldap/Ad) fixes group sync option See merge request tine20/tine20!5710
tine-groupware · Jul 31, 2024 · 094a25f · 094a25f
2 parents 73c7662 + b99e550
commit 094a25f
Show file tree

Hide file tree

Showing 9 changed files with 99 additions and 64 deletions.
diff --git a/tine20/Tinebase/Config.php b/tine20/Tinebase/Config.php
@@ -464,7 +464,7 @@ class Tinebase_Config extends Tinebase_Config_Abstract
     const SYNC_USER_DISABLED = 'syncUserDisabled';
 
     const SYNC_USER_OF_GROUPS = 'syncUserOfGroups';
-    const SYNC_DEVIATED_PRIMARY_GROUP = 'syncDeviatedPrimaryGroup';
+    const SYNC_DEVIATED_PRIMARY_GROUP_UUID = 'syncDeviatedPrimaryGroupUUID';
 
     /**
      * configure if user contact data should be synced from sync backend, default yes
@@ -1767,7 +1767,7 @@ class Tinebase_Config extends Tinebase_Config_Abstract
                             self::SETBYSETUPMODULE => false,
                             self::DEFAULT_STR => [],
                         ],
-                        self::SYNC_DEVIATED_PRIMARY_GROUP => [
+                        self::SYNC_DEVIATED_PRIMARY_GROUP_UUID => [
                             //_('Sync deviated user accounts default group')
                             self::LABEL => 'Sync deviated user accounts default group',
                             //_('Sync deviated user accounts default group')

diff --git a/tine20/Tinebase/Group/ActiveDirectory.php b/tine20/Tinebase/Group/ActiveDirectory.php
@@ -118,7 +118,7 @@ public function __construct(array $_options)
 
     public function addGroupInSyncBackend(Tinebase_Model_Group $_group): ?Tinebase_Model_Group
     {
-        if ($this->isDisabledBackend() || !$this->getWriteableGroupIds([$_group->getId()])) {
+        if ($this->isDisabledBackend() || (!empty($groupNames = Tinebase_Config::getInstance()->{Tinebase_Config::USERBACKEND}->{Tinebase_Config::SYNCOPTIONS}->{Tinebase_Config::SYNC_USER_OF_GROUPS})) && !in_array($_group->name, $groupNames)) {
             return null;
         }
 
@@ -286,7 +286,7 @@ public function updateGroupInSyncBackend(Tinebase_Model_Group $_group)
         if ($this->isDisabledBackend() || !$this->getWriteableGroupIds([$_group->getId()])) {
             return $_group;
         }
-        
+
         $metaData = $this->_getMetaData($_group->getId());
         $dn = $metaData['dn'];
 
@@ -322,7 +322,7 @@ public function updateGroupInSyncBackend(Tinebase_Model_Group $_group)
         if (null !== $this->_writeGroupsIds) {
             $this->setGroupMembersInSyncBackend($_group->getId(), $this->getGroupMembers($_group->getId()));
         }
-        
+
         $group = $this->getGroupByIdFromSyncBackend($_group);
 
         return $group;
@@ -492,6 +492,19 @@ public function setGroupMembersInSyncBackend($_groupId, $_groupMembers)
         $_groupId = $_groupId[0];
 
         $groupMetaData = $this->_getMetaData($_groupId);
+
+        $removedAccounts = [];
+        if ($this->_writeGroupsIds) {
+            foreach ((array)$_groupMembers as $userId) {
+                // make sure the account exists in sync backend
+                /** @var Tinebase_User_Interface_SyncAble $syncCtrl */
+                $syncCtrl = Tinebase_User::getInstance();
+                $syncCtrl->setUserAsWriteGroupMember($userId);
+                Tinebase_User::getInstance()->updateUser(Tinebase_User::getInstance()->getFullUserById($userId));
+            }
+
+            $removedAccounts = array_diff($this->getGroupMembers($_groupId), $_groupMembers);
+        }
 
         $membersMetaDatas = $this->_getAccountsMetaData((array)$_groupMembers, FALSE);
         if (count($_groupMembers) !== count($membersMetaDatas)) {
@@ -503,18 +516,6 @@ public function setGroupMembersInSyncBackend($_groupId, $_groupMembers)
                 $_groupMembers[] = $account[$this->_userUUIDAttribute];
             }
         }
-
-        $removedAccounts = [];
-        if ($this->_writeGroupsIds) {
-            foreach ((array)$_groupMembers as $userId) {
-                // make sure the account exists in sync backend
-                /** @var Tinebase_User_Interface_SyncAble $syncAble */
-                $syncAble = Tinebase_User::getInstance();
-                $syncAble->updateUserInSyncBackend(Tinebase_User::getInstance()->getFullUserById($userId));
-            }
-
-            $removedAccounts = array_diff($this->getGroupMembers($_groupId), $_groupMembers);
-        }
 
         if (Tinebase_Core::isLogLevel(Zend_Log::TRACE)) 
             Tinebase_Core::getLogger()->trace(__METHOD__ . '::' . __LINE__ . '  $group data: ' . print_r($groupMetaData, true));

diff --git a/tine20/Tinebase/Group/Ldap.php b/tine20/Tinebase/Group/Ldap.php
@@ -126,14 +126,6 @@ public function __construct(array $_options)
         if (Tinebase_Config::getInstance()->{Tinebase_Config::USERBACKEND}->{Tinebase_Config::SYNCOPTIONS}->{Tinebase_Config::SYNC_USER_DISABLED}) {
             $this->_isReadOnlyBackend = true;
         }
-        if ($groupNames = Tinebase_Config::getInstance()->{Tinebase_Config::USERBACKEND}->{Tinebase_Config::SYNCOPTIONS}->{Tinebase_Config::SYNC_USER_OF_GROUPS}) {
-            $this->_writeGroupsIds = [];
-            foreach ($groupNames as $groupName) {
-                try {
-                    $this->_writeGroupsIds[] = Tinebase_Group::getInstance()->getGroupByName($groupName)->getId();
-                } catch (Tinebase_Exception_Record_NotDefined) {}
-            }
-        }
         if ((isset($_options['ldap']) || array_key_exists('ldap', $_options))) {
             $this->_ldap = $_options['ldap'];
         }
@@ -156,6 +148,15 @@ public function __construct(array $_options)
                 $this->_plugins[$className] = new $className($this->getLdap(), $this->_options);
             }
         }
+
+        if ($groupNames = Tinebase_Config::getInstance()->{Tinebase_Config::USERBACKEND}->{Tinebase_Config::SYNCOPTIONS}->{Tinebase_Config::SYNC_USER_OF_GROUPS}) {
+            $this->_writeGroupsIds = [];
+            foreach ($groupNames as $groupName) {
+                try {
+                    $this->_writeGroupsIds[] = $this->getGroupByName($groupName)->getId();
+                } catch (Tinebase_Exception_Record_NotDefined) {}
+            }
+        }
     }
 
     /**
@@ -302,6 +303,19 @@ public function setGroupMembersInSyncBackend($_groupId, $_groupMembers)
 
         $metaData = $this->_getMetaData($_groupId);
 
+        $removedAccounts = [];
+        if ($this->_writeGroupsIds) {
+            foreach ((array)$_groupMembers as $userId) {
+                // make sure the account exists in sync backend
+                /** @var Tinebase_User_Interface_SyncAble $syncCtrl */
+                $syncCtrl = Tinebase_User::getInstance();
+                $syncCtrl->setUserAsWriteGroupMember($userId);
+                Tinebase_User::getInstance()->updateUser(Tinebase_User::getInstance()->getFullUserById($userId));
+            }
+
+            $removedAccounts = array_diff($this->getGroupMembers($_groupId), $_groupMembers);
+        }
+
         $membersMetaDatas = $this->_getAccountsMetaData((array)$_groupMembers, FALSE);
         if (count($_groupMembers) !== count($membersMetaDatas)) {
             if (Tinebase_Core::isLogLevel(Zend_Log::INFO)) Tinebase_Core::getLogger()->info(__METHOD__ . '::' . __LINE__
@@ -312,18 +326,6 @@ public function setGroupMembersInSyncBackend($_groupId, $_groupMembers)
                 $_groupMembers[] = $account[$this->_userUUIDAttribute];
             }
         }
-
-        $removedAccounts = [];
-        if ($this->_writeGroupsIds) {
-            foreach ((array)$_groupMembers as $userId) {
-                // make sure the account exists in sync backend
-                /** @var Tinebase_User_Interface_SyncAble $syncAble */
-                $syncAble = Tinebase_User::getInstance();
-                $syncAble->updateUserInSyncBackend(Tinebase_User::getInstance()->getFullUserById($userId));
-            }
-
-            $removedAccounts = array_diff($this->getGroupMembers($_groupId), $_groupMembers);
-        }
 
         if (Tinebase_Core::isLogLevel(Zend_Log::TRACE)) 
             Tinebase_Core::getLogger()->trace(__METHOD__ . '::' . __LINE__ . '  $group data: ' . print_r($metaData, true));
@@ -600,7 +602,7 @@ public function removeGroupMemberInSyncBackend($_groupId, $_accountId, $_checkWr
 
     public function addGroupInSyncBackend(Tinebase_Model_Group $_group): ?Tinebase_Model_Group
     {
-        if ($this->isDisabledBackend() || !$this->getWriteableGroupIds([$_group->getId()])) {
+        if ($this->isDisabledBackend() || (!empty($groupNames = Tinebase_Config::getInstance()->{Tinebase_Config::USERBACKEND}->{Tinebase_Config::SYNCOPTIONS}->{Tinebase_Config::SYNC_USER_OF_GROUPS})) && !in_array($_group->name, $groupNames)) {
             return null;
         }
 
@@ -654,7 +656,7 @@ public function updateGroupInSyncBackend(Tinebase_Model_Group $_group)
         if ($this->isDisabledBackend() || !$this->getWriteableGroupIds([$_group->getId()])) {
             return $_group;
         }
-        
+
         $metaData = $this->_getMetaData($_group->getId());
         $dn = $metaData['dn'];
 

diff --git a/tine20/Tinebase/Group/Sql.php b/tine20/Tinebase/Group/Sql.php
@@ -194,9 +194,9 @@ public function setGroupMembers($_groupId, $_groupMembers)
             . ' Setting ' . count($_groupMembers) . ' new groupmembers for group ' . $_groupId);
 
         if ($this instanceof Tinebase_Group_Interface_SyncAble) {
-            $_groupMembers = $this->setGroupMembersInSyncBackend($_groupId, $_groupMembers);
+            $this->setGroupMembersInSyncBackend($_groupId, $_groupMembers);
         }
-        
+
         $this->setGroupMembersInSqlBackend($_groupId, $_groupMembers);
     }
 

diff --git a/tine20/Tinebase/User.php b/tine20/Tinebase/User.php
@@ -492,6 +492,10 @@ protected static function _getLdapUserController()
      */
     public static function syncUser($username, $options = array())
     {
+        if (Tinebase_Config::getInstance()->{Tinebase_Config::USERBACKEND}->{Tinebase_Config::SYNCOPTIONS}->{Tinebase_Config::SYNC_USER_DISABLED}) {
+            return null;
+        }
+
         if ($username instanceof Tinebase_Model_FullUser) {
             $username = $username->accountLoginName;
         }

diff --git a/tine20/Tinebase/User/ActiveDirectory.php b/tine20/Tinebase/User/ActiveDirectory.php
@@ -124,6 +124,7 @@ public function addUserToSyncBackend(Tinebase_Model_FullUser $_user)
         }
 
         $ldapData = $this->_user2ldap($_user);
+        unset($ldapData[$this->_userUUIDAttribute]);
 
         $ldapData = array_merge($ldapData, $this->getLdapPasswordData(Tinebase_Record_Abstract::generateUID(20)));
 
@@ -150,13 +151,15 @@ public function addUserToSyncBackend(Tinebase_Model_FullUser $_user)
 
         // add user to primary group and set primary group
         /** @noinspection PhpUndefinedMethodInspection */
-        Tinebase_Group::getInstance()->addGroupMemberInSyncBackend(Tinebase_Config::getInstance()->{Tinebase_Config::USERBACKEND}->{Tinebase_Config::SYNCOPTIONS}->{Tinebase_Config::SYNC_DEVIATED_PRIMARY_GROUP} ?: $_user->accountPrimaryGroup, $userId);
+        Tinebase_Group::getInstance()->addGroupMemberInSyncBackend(Tinebase_Config::getInstance()->{Tinebase_Config::USERBACKEND}->{Tinebase_Config::SYNCOPTIONS}->{Tinebase_Config::SYNC_DEVIATED_PRIMARY_GROUP_UUID} ?: $_user->accountPrimaryGroup, $userId);
 
         // set primary group id
         $this->_ldap->updateProperty($dn, array('primarygroupid' => $primaryGroupId));
-
 
-        $user = $this->getUserByPropertyFromSyncBackend('accountId', $userId, 'Tinebase_Model_FullUser');
+        $user = clone $_user;
+        $user->setId($userId);
+        unset($user->xprops()[static::class]['syncId']);
+        $user = $this->getUserByPropertyFromSyncBackend('accountId', $user, 'Tinebase_Model_FullUser');
 
         return $user;
     }
@@ -253,19 +256,18 @@ public function setPassword($_userId, $_password, $_encrypt = TRUE, $_mustChange
             Tinebase_Core::getLogger()->trace(__METHOD__ . '::' . __LINE__ . '  $ldapData: ' . print_r($ldapData, true));
 
         $this->_ldap->updateProperty($metaData['dn'], $ldapData);
-
-        // update last modify timestamp in sql backend too
-        $values = array(
-            'last_password_change' => Tinebase_DateTime::now()->get(Tinebase_Record_Abstract::ISO8601LONG),
-            'password_must_change' => 0,
-        );
-
-        $where = array(
-            $this->_db->quoteInto($this->_db->quoteIdentifier('id') . ' = ?', $user->getId())
-        );
-
-        $this->_db->update(SQL_TABLE_PREFIX . 'accounts', $values, $where);
-
+
+        if ($this->_options[Tinebase_Config::USERBACKEND_WRITE_PW_TO_SQL]) {
+            $this->_updatePasswordProperties($user->getId(), $_password, $_encrypt, $_mustChange);
+        } else {
+            try {
+                // update last modify timestamp in sql backend too
+                $this->_setAccountPasswordProperties($user->getId());
+            } catch (Tinebase_Exception_NotFound $tenf) {
+                if (Tinebase_Core::isLogLevel(Zend_Log::NOTICE)) Tinebase_Core::getLogger()->notice(
+                    __METHOD__ . '::' . __LINE__ . ' ' . $tenf);
+            }
+        }
         $this->_setPluginsPassword($user, $_password, $_encrypt);
 
         $this->firePasswordEvent($user, $_password);
@@ -582,7 +584,7 @@ protected function _user2ldap(Tinebase_Model_FullUser $_user, array $_ldapEntry
                     break;
 
                 case 'accountPrimaryGroup':
-                    if ($deviateGroupId = Tinebase_Config::getInstance()->{Tinebase_Config::USERBACKEND}->{Tinebase_Config::SYNCOPTIONS}->{Tinebase_Config::SYNC_DEVIATED_PRIMARY_GROUP}) {
+                    if ($deviateGroupId = Tinebase_Config::getInstance()->{Tinebase_Config::USERBACKEND}->{Tinebase_Config::SYNCOPTIONS}->{Tinebase_Config::SYNC_DEVIATED_PRIMARY_GROUP_UUID}) {
                         $value = $deviateGroupId;
                     }
                     /** @noinspection PhpUndefinedMethodInspection */

diff --git a/tine20/Tinebase/User/Interface/SyncAble.php b/tine20/Tinebase/User/Interface/SyncAble.php
@@ -97,4 +97,6 @@ public function updateContactFromSyncBackend(Tinebase_Model_FullUser $_user, Add
      * @param Addressbook_Model_Contact $_contact
      */
     public function updateContactInSyncBackend($_contact);
+
+    public function setUserAsWriteGroupMember(string $userId, bool $value = true): void;
 }
diff --git a/tine20/Tinebase/User/Ldap.php b/tine20/Tinebase/User/Ldap.php
@@ -364,6 +364,9 @@ public function getUserByPropertyFromSyncBackend($_property, $_accountId, $_acco
             throw new Tinebase_Exception_NotFound("can't get user by property $_property. property not supported by ldap backend.");
         }
 
+        if ('accountId' === $_property && ! $_accountId instanceof Tinebase_Model_FullUser) {
+            $_accountId = $this->getFullUserById($_accountId);
+        }
         $ldapEntry = $this->_getLdapEntry($_property, $_accountId);
 
         $user = $this->_ldap2User($ldapEntry, $_accountClass);
@@ -634,6 +637,7 @@ public function addUserToSyncBackend(Tinebase_Model_FullUser $user)
         }
 
         $ldapData = $this->_user2ldap($user);
+        unset($ldapData[$this->_userUUIDAttribute]);
 
         $ldapData['uidnumber'] = $this->_generateUidNumber();
         $ldapData['objectclass'] = $this->_requiredObjectClass;
@@ -653,7 +657,10 @@ public function addUserToSyncBackend(Tinebase_Model_FullUser $user)
 
         $userId = $userId[$this->_userUUIDAttribute][0];
 
-        $user = $this->getUserByPropertyFromSyncBackend('accountId', $userId, 'Tinebase_Model_FullUser');
+        $user = clone $user;
+        $user->setId($userId);
+        unset($user->xprops()[static::class]['syncId']);
+        $user = $this->getUserByPropertyFromSyncBackend('accountId', $user, 'Tinebase_Model_FullUser');
 
         return $user;
     }
@@ -721,14 +728,18 @@ protected function _getLdapEntry($_property, $_userId)
     {
         switch($_property) {
             case 'accountId':
-                $value = $this->_encodeAccountId(Tinebase_Model_User::convertUserIdToInt($_userId));
+                if ($_userId instanceof Tinebase_Model_User && ($_userId->xprops()[static::class]['syncId'] ?? false)) {
+                    $value = $this->_encodeAccountId($_userId->xprops()[static::class]['syncId']);
+                } else {
+                    $value = $this->_encodeAccountId(Tinebase_Model_User::convertUserIdToInt($_userId));
+                }
                 break;
             default:
                 /** @noinspection PhpDeprecationInspection */
                 $value = Zend_Ldap::filterEscape($_userId);
                 break;
         }
-        
+
         $filter = Zend_Ldap_Filter::andFilter(
             Zend_Ldap_Filter::string($this->_userBaseFilter),
             Zend_Ldap_Filter::equals($this->_rowNameMapping[$_property], $value)
@@ -772,7 +783,12 @@ protected function _getLdapEntry($_property, $_userId)
      */
     protected function _getMetaData($_userId)
     {
-        $userId = $this->_encodeAccountId(Tinebase_Model_User::convertUserIdToInt($_userId));
+
+        if (!$_userId instanceof Tinebase_Model_User) {
+            $_userId = $this->getFullUserById($_userId);
+        }
+
+        $userId = $this->_encodeAccountId($_userId->xprops()[static::class]['syncId'] ?? Tinebase_Model_User::convertUserIdToInt($_userId));
 
         $filter = Zend_Ldap_Filter::equals(
             $this->_rowNameMapping['accountId'], $userId
@@ -1152,7 +1168,7 @@ protected function _user2ldap(Tinebase_Model_FullUser $_user, array $_ldapEntry
                         $ldapData = array_merge($ldapData, $this->_getUserStatusValues($value));
                         break;
                     case 'accountPrimaryGroup':
-                        if ($deviateGroupId = Tinebase_Config::getInstance()->{Tinebase_Config::USERBACKEND}->{Tinebase_Config::SYNCOPTIONS}->{Tinebase_Config::SYNC_DEVIATED_PRIMARY_GROUP}) {
+                        if ($deviateGroupId = Tinebase_Config::getInstance()->{Tinebase_Config::USERBACKEND}->{Tinebase_Config::SYNCOPTIONS}->{Tinebase_Config::SYNC_DEVIATED_PRIMARY_GROUP_UUID}) {
                             $value = $deviateGroupId;
                         }
                         /** @var Tinebase_Group_Ldap $groupController */
@@ -1252,4 +1268,11 @@ protected function isReadOnlyUser(string|int|null $userId): bool
         }
         return false;
     }
+
+    public function setUserAsWriteGroupMember(string $userId, bool $value = true): void
+    {
+        // to make sure that the cache is initialized
+        $this->isReadOnlyUser($userId);
+        $this->_writeGroupsMembers[$userId] = !$value;
+    }
 }
diff --git a/tine20/Tinebase/User/Sql.php b/tine20/Tinebase/User/Sql.php
@@ -873,7 +873,8 @@ public function updateUser(Tinebase_Model_FullUser $_user)
             try {
                 $this->updateUserInSyncBackend($_user);
             } catch (Tinebase_Exception_NotFound) {
-                $this->addUserToSyncBackend($_user);
+                $createdSyncUser = $this->addUserToSyncBackend($_user);
+                $_user->xprops()[static::class]['syncId'] = $createdSyncUser->getId();
             }
         }