Skip to content

Commit

Permalink
tweak(TB AD/Ldap) fix syncGroup id split brain
Browse files Browse the repository at this point in the history
  • Loading branch information
@paulmhh
paulmhh committed Oct 2, 2024
1 parent 71a2336 commit 042bf20
Show file tree
Hide file tree
Showing 8 changed files with 73 additions and 32 deletions.
9 changes: 9 additions & 0 deletions tine20/Tinebase/Config.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,8 @@ class Tinebase_Config extends Tinebase_Config_Abstract
*/
const SYNCOPTIONS = 'syncOptions';

const PWD_CANT_CHANGE = 'pwdCantChange';

/**
* user backend writes user pw to sql
*
Expand Down Expand Up @@ -1707,6 +1709,13 @@ class Tinebase_Config extends Tinebase_Config_Abstract
self::TYPE => 'object',
self::CLASSNAME => Tinebase_Config_Struct::class,
self::CONTENT => array(
self::PWD_CANT_CHANGE => [
self::TYPE => self::TYPE_BOOL,
self::CLIENTREGISTRYINCLUDE => false,
self::SETBYADMINMODULE => false,
self::SETBYSETUPMODULE => false,
self::DEFAULT_STR => false
],
self::SYNC_USER_CONTACT_DATA => array(
//_('Sync contact data from sync backend')
self::LABEL => 'Sync contact data from sync backend',
Expand Down
25 changes: 20 additions & 5 deletions tine20/Tinebase/Group/ActiveDirectory.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,15 +165,23 @@ public function addGroupInSyncBackend(Tinebase_Model_Group $_group): ?Tinebase_M
* @param mixed $_groupId
* @param mixed $_accountId string or user object
*/
public function addGroupMemberInSyncBackend($_groupId, $_accountId)
public function addGroupMemberInSyncBackend($_groupId, $_accountId, $_checkExistance = true)
{
if ($this->isDisabledBackend() || !($groupId = $this->getWriteableGroupIds([Tinebase_Model_Group::convertGroupIdToInt($_groupId)]))) {
return;
}
$groupId = $groupId[0];
$userId = Tinebase_Model_User::convertUserIdToInt($_accountId);

if ($this->_writeGroupsIds && $_checkExistance) {
// 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));
}

$memberships = $this->getGroupMembershipsFromSyncBackend($userId);
$memberships = $this->getGroupMembershipsFromSyncBackend($_accountId);
if (in_array($groupId, $memberships)) {
if (Tinebase_Core::isLogLevel(Zend_Log::DEBUG))
Tinebase_Core::getLogger()->debug(__METHOD__ . '::' . __LINE__ . " skip adding group member, as $userId is already in group $groupId");
Expand All @@ -182,7 +190,7 @@ public function addGroupMemberInSyncBackend($_groupId, $_accountId)
}

$groupDn = $this->_getDn($groupId);
$accountMetaData = $this->_getAccountMetaData($userId);
$accountMetaData = $this->_getAccountMetaData($_accountId);

if (Tinebase_Core::isLogLevel(Zend_Log::TRACE))
Tinebase_Core::getLogger()->trace(__METHOD__ . '::' . __LINE__ . " account meta data: " . print_r($accountMetaData, true));
Expand All @@ -206,7 +214,10 @@ public function getGroupMembershipsFromSyncBackend($_userId)
return [];
}

$userId = $_userId instanceof Tinebase_Model_User ? $_userId->getId() : $_userId;
if ($this->_writeGroupsIds && !$_userId instanceof Tinebase_Model_FullUser) {
$_userId = Tinebase_User::getInstance()->getUserByPropertyFromSqlBackend('accountId', $_userId, Tinebase_Model_FullUser::class);
}
$userId = $_userId instanceof Tinebase_Model_User ? ($_userId->xprops()[Tinebase_User::getInstance()::class]['syncId'] ?? $_userId->geTId()) : $_userId;

// find user in AD and retrieve memberOf attribute
$filter = Zend_Ldap_Filter::andFilter(
Expand Down Expand Up @@ -631,7 +642,11 @@ protected function _getAccountsMetaData(array $_accountIds, $throwExceptionOnMis
{
$filterArray = array();
foreach ($_accountIds as $accountId) {
$accountId = Tinebase_Model_User::convertUserIdToInt($accountId);
if ($this->_writeGroupsIds && !$accountId instanceof Tinebase_Model_FullUser) {
$accountId = Tinebase_User::getInstance()->getUserByPropertyFromSqlBackend('accountId', $accountId, Tinebase_Model_FullUser::class);
}
$accountId = $accountId instanceof Tinebase_Model_User ? ($accountId->xprops()[Tinebase_User::getInstance()::class]['syncId'] ?? $accountId->getId()) : Tinebase_Model_User::convertUserIdToInt($accountId);

$filterArray[] = Zend_Ldap_Filter::equals($this->_userUUIDAttribute, $this->_encodeAccountId($accountId));
}
$filter = new Zend_Ldap_Filter_Or($filterArray);
Expand Down
2 changes: 1 addition & 1 deletion tine20/Tinebase/Group/Interface/SyncAble.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public function setGroupMembersInSyncBackend($_groupId, $_groupMembers);
* @param mixed $_groupId
* @param mixed $_accountId string or user object
*/
public function addGroupMemberInSyncBackend($_groupId, $_accountId);
public function addGroupMemberInSyncBackend($_groupId, $_accountId, $_checkExistance = true);

/**
* remove one member from the group in sync backend
Expand Down
29 changes: 19 additions & 10 deletions tine20/Tinebase/Group/Ldap.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,7 @@ public function setGroupMembershipsInSyncBackend($_userId, $_groupIds)
// make sure the account exists in sync backend
/** @var Tinebase_User_Interface_SyncAble $syncAble */
$syncAble = Tinebase_User::getInstance();
$syncAble->setUserAsWriteGroupMember($userId);
$syncAble->updateUserInSyncBackend(Tinebase_User::getInstance()->getFullUserById($userId));
}

Expand All @@ -413,7 +414,7 @@ public function setGroupMembershipsInSyncBackend($_userId, $_groupIds)
if (Tinebase_Core::isLogLevel(Zend_Log::TRACE)) Tinebase_Core::getLogger()->trace(__METHOD__ . '::' . __LINE__ . ' removed groupmemberships: ' . print_r($removeGroupMemberships, true));

foreach ($addGroupMemberships as $groupId) {
$this->addGroupMemberInSyncBackend($groupId, $userId);
$this->addGroupMemberInSyncBackend($groupId, $userId, false);
}

foreach ($removeGroupMemberships as $groupId) {
Expand All @@ -426,22 +427,24 @@ public function setGroupMembershipsInSyncBackend($_userId, $_groupIds)
/**
* add a new groupmember to group in sync backend
*
* @param mixed $_groupId
* @param mixed $_accountId string or user object
* @param mixed $_groupId
* @param mixed $_accountId
* @param true $_checkExistance
*/
public function addGroupMemberInSyncBackend($_groupId, $_accountId)
public function addGroupMemberInSyncBackend($_groupId, $_accountId, $_checkExistance = true)
{
if ($this->isDisabledBackend() || !($groupId = $this->getWriteableGroupIds([Tinebase_Model_Group::convertGroupIdToInt($_groupId)]))) {
return;
}
$groupId = $groupId[0];
$userId = Tinebase_Model_User::convertUserIdToInt($_accountId);

if ($this->_writeGroupsIds) {
if ($this->_writeGroupsIds && $_checkExistance) {
// 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));
/** @var Tinebase_User_Interface_SyncAble $syncCtrl */
$syncCtrl = Tinebase_User::getInstance();
$syncCtrl->setUserAsWriteGroupMember($userId);
$syncCtrl->updateUserInSyncBackend(Tinebase_User::getInstance()->getFullUserById($userId));
}

$memberships = $this->getGroupMembershipsFromSyncBackend($_accountId);
Expand Down Expand Up @@ -781,7 +784,10 @@ protected function _getMetaData($_groupId)
*/
protected function _getUserMetaData($_userId)
{
$userId = $this->_encodeAccountId(Tinebase_Model_User::convertUserIdToInt($_userId));
if ($this->_writeGroupsIds && !$_userId instanceof Tinebase_Model_FullUser) {
$_userId = Tinebase_User::getInstance()->getUserByPropertyFromSqlBackend('accountId', $_userId, Tinebase_Model_FullUser::class);
}
$userId = $_userId instanceof Tinebase_Model_User ? ($_userId->xprops()[Tinebase_User::getInstance()::class]['syncId'] ?? $_userId->getId()) : Tinebase_Model_User::convertUserIdToInt($_userId);

$filter = Zend_Ldap_Filter::equals(
$this->_userUUIDAttribute, $userId
Expand Down Expand Up @@ -812,7 +818,10 @@ protected function _getAccountsMetaData(array $_accountIds, $throwExceptionOnMis
{
$filterArray = array();
foreach ($_accountIds as $accountId) {
$accountId = Tinebase_Model_User::convertUserIdToInt($accountId);
if ($this->_writeGroupsIds && !$accountId instanceof Tinebase_Model_FullUser) {
$accountId = Tinebase_User::getInstance()->getUserByPropertyFromSqlBackend('accountId', $accountId, Tinebase_Model_FullUser::class);
}
$accountId = $accountId instanceof Tinebase_Model_User ? ($accountId->xprops()[Tinebase_User::getInstance()::class]['syncId'] ?? $accountId->getId()) : Tinebase_Model_User::convertUserIdToInt($accountId);
$filterArray[] = Zend_Ldap_Filter::equals($this->_userUUIDAttribute, Zend_Ldap::filterEscape($accountId));
}
$filter = new Zend_Ldap_Filter_Or($filterArray);
Expand Down
29 changes: 17 additions & 12 deletions tine20/Tinebase/User/ActiveDirectory.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,13 @@ public function __construct(array $_options = array())
public function addUserToSyncBackend(Tinebase_Model_FullUser $_user)
{
if ($this->isReadOnlyUser($_user->getId())) {
return NULL;
return null;
}

$ldapData = $this->_user2ldap($_user);
unset($ldapData[$this->_userUUIDAttribute]);
if ($this->_writeGroupsIds) {
unset($ldapData[$this->_userUUIDAttribute]);
}

$ldapData = array_merge($ldapData, $this->getLdapPasswordData(Tinebase_Record_Abstract::generateUID(20)));

Expand All @@ -143,25 +145,25 @@ public function addUserToSyncBackend(Tinebase_Model_FullUser $_user)

$dn = $this->generateDn($_user);

if (Tinebase_Core::isLogLevel(Zend_Log::TRACE))
Tinebase_Core::getLogger()->trace(__METHOD__ . '::' . __LINE__ . ' ldapData: ' . print_r($ldapData, true));
if (Tinebase_Core::isLogLevel(Zend_Log::DEBUG))
Tinebase_Core::getLogger()->debug(__METHOD__ . '::' . __LINE__ . ' ldapData: ' . print_r($ldapData, true));

$this->_ldap->add($dn, $ldapData);

$userId = $this->_ldap->getEntry($dn, array($this->_userUUIDAttribute));
$userId = $this->_decodeAccountId($userId[$this->_userUUIDAttribute][0]);

if ($this->_writeGroupsIds) {
$_user->xprops()[static::class]['syncId'] = $userId;
}

// 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_UUID} ?: $_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, $_user, false);

// set primary group id
$this->_ldap->updateProperty($dn, array('primarygroupid' => $primaryGroupId));

$user = clone $_user;
$user->setId($userId);
unset($user->xprops()[static::class]['syncId']);
$user = $this->getUserByPropertyFromSyncBackend('accountId', $user, 'Tinebase_Model_FullUser');
$user = $this->getUserByPropertyFromSyncBackend('accountId', $_user, 'Tinebase_Model_FullUser');

return $user;
}
Expand Down Expand Up @@ -347,7 +349,7 @@ public function updateUserInSyncBackend(Tinebase_Model_FullUser $_account)
}

/** @noinspection PhpUndefinedMethodInspection */
Tinebase_Group::getInstance()->addGroupMemberInSyncBackend($_account->accountPrimaryGroup, $_account->getId());
Tinebase_Group::getInstance()->addGroupMemberInSyncBackend($_account->accountPrimaryGroup, $_account->getId(), false);

$ldapEntry = $this->_getLdapEntry('accountId', $_account);

Expand Down Expand Up @@ -442,7 +444,7 @@ protected function _encodeAccountId($accountId)
*/
public function generateDn(Tinebase_Model_FullUser $_account)
{
$newDn = "cn={$_account->accountFullName},{$this->_baseDn}";
$newDn = 'cn=' . Zend_Ldap_Filter_Abstract::escapeValue($_account->accountFullName) . ",{$this->_baseDn}";

return $newDn;
}
Expand Down Expand Up @@ -555,6 +557,9 @@ protected function _user2ldap(Tinebase_Model_FullUser $_user, array $_ldapEntry
$ldapData = array(
'useraccountcontrol' => isset($_ldapEntry['useraccountcontrol']) ? $_ldapEntry['useraccountcontrol'][0] : self::NORMAL_ACCOUNT
);
if (Tinebase_Config::getInstance()->{Tinebase_Config::USERBACKEND}->{Tinebase_Config::SYNCOPTIONS}->{Tinebase_Config::PWD_CANT_CHANGE}) {
$ldapData['useraccountcontrol'] &= self::PASSWD_CANT_CHANGE;
}

if (isset($_user->xprops()['uidnumber'])) {
$_ldapEntry['uidnumber'] = $_user->xprops()['uidnumber'];
Expand Down
2 changes: 2 additions & 0 deletions tine20/Tinebase/User/Interface/SyncAble.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ public function setExpiryDateInSyncBackend($_accountId, $_expiryDate);
*/
public function addUserToSyncBackend(Tinebase_Model_FullUser $_user);

public function generateDn(Tinebase_Model_FullUser $_account);

/**
* updates an existing user
*
Expand Down
8 changes: 4 additions & 4 deletions tine20/Tinebase/User/Ldap.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ public function updateUserInSyncBackend(Tinebase_Model_FullUser $_account)

// add the user to current groups again
foreach ($memberships as $groupId) {
$groupsBackend->addGroupMemberInSyncBackend($groupId, $_account);
$groupsBackend->addGroupMemberInSyncBackend($groupId, $_account, false);
}
}

Expand Down Expand Up @@ -778,11 +778,11 @@ protected function _getLdapEntry($_property, $_userId)
protected function _getMetaData($_userId)
{

if (!$_userId instanceof Tinebase_Model_User) {
if ($this->_writeGroupsIds && !$_userId instanceof Tinebase_Model_FullUser) {
$_userId = $this->getFullUserById($_userId);
}

$userId = $this->_encodeAccountId($_userId->xprops()[static::class]['syncId'] ?? Tinebase_Model_User::convertUserIdToInt($_userId));
$userId = $this->_encodeAccountId($_userId instanceof Tinebase_Model_FullUser ? ($_userId->xprops()[static::class]['syncId'] ?? $_userId->getId()) : Tinebase_Model_User::convertUserIdToInt($_userId));

$filter = Zend_Ldap_Filter::equals(
$this->_rowNameMapping['accountId'], $userId
Expand Down Expand Up @@ -819,7 +819,7 @@ public function generateDn(Tinebase_Model_FullUser $_account)
{
$baseDn = $this->_baseDn;
$uidProperty = array_search('uid', $this->_rowNameMapping);
$newDn = "uid={$_account->$uidProperty},{$baseDn}";
$newDn = 'uid=' . Zend_Ldap_Filter_Abstract::escapeValue($_account->$uidProperty) . ",{$baseDn}";

return $newDn;
}
Expand Down
1 change: 1 addition & 0 deletions tine20/Tinebase/User/Sql.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -875,6 +875,7 @@ public function updateUser(Tinebase_Model_FullUser $_user)
try {
$this->updateUserInSyncBackend($_user);
} catch (Tinebase_Exception_NotFound) {
if (Tinebase_Core::isLogLevel(Zend_Log::DEBUG)) Tinebase_Core::getLogger()->debug(__METHOD__ . '::' . __LINE__ . ' backend user not found, creating instead of updating');
$createdSyncUser = $this->addUserToSyncBackend($_user);
$_user->xprops()[static::class]['syncId'] = $createdSyncUser->getId();
}
Expand Down

0 comments on commit 042bf20

Please sign in to comment.