0012116: ldap sync: allow empty ldap values to overwrite contact values
authorPhilipp Schüle <p.schuele@metaways.de>
Wed, 17 Aug 2016 12:40:57 +0000 (14:40 +0200)
committerPhilipp Schüle <p.schuele@metaways.de>
Wed, 17 Aug 2016 13:23:49 +0000 (15:23 +0200)
* contact data is not updated during ldap sync
* create test that reproduces the problem
* allows to define synced (overwritten) fields
 in config

https://forge.tine20.org/view.php?id=12116

Change-Id: Icd0ee745e65ef075ec8d9ec31fc222c855a12b4d
Reviewed-on: http://gerrit.tine20.com/customers/3446
Tested-by: Jenkins CI (http://ci.tine20.com/)
Reviewed-by: Philipp Schüle <p.schuele@metaways.de>
tests/tine20/Tinebase/User/LdapTest.php
tine20/Tinebase/Config.php
tine20/Tinebase/User.php
tine20/Tinebase/User/ActiveDirectory.php
tine20/Tinebase/User/Ldap.php

index 6bb68eb..56d69ff 100644 (file)
@@ -5,16 +5,11 @@
  * @package     Tinebase
  * @subpackage  User
  * @license     http://www.gnu.org/licenses/agpl.html
- * @copyright   Copyright (c) 2008-2014 Metaways Infosystems GmbH (http://www.metaways.de)
+ * @copyright   Copyright (c) 2008-2016 Metaways Infosystems GmbH (http://www.metaways.de)
  * @author      Lars Kneschke <l.kneschke@metaways.de>
  */
 
 /**
- * Test helper
- */
-require_once dirname(dirname(dirname(__FILE__))) . DIRECTORY_SEPARATOR . 'TestHelper.php';
-
-/**
  * Test class for Tinebase_User_Ldap
  */
 class Tinebase_User_LdapTest extends TestCase
@@ -38,8 +33,20 @@ class Tinebase_User_LdapTest extends TestCase
             $this->markTestSkipped('LDAP backend not enabled');
         }
         $this->_backend = Tinebase_User::factory(Tinebase_User::LDAP);
+
+        parent::setUp();
     }
-    
+
+    /**
+     * tear down tests
+     */
+    protected function tearDown()
+    {
+        Tinebase_Config::getInstance()->set(Tinebase_Config::LDAP_OVERWRITE_CONTACT_FIELDS, array());
+
+        parent::tearDown();
+    }
+
     /**
      * try to add an user
      * 
@@ -216,14 +223,58 @@ class Tinebase_User_LdapTest extends TestCase
     
     /**
      * execute Tinebase_User::syncUser
+     *
+     * TODO test bday
      */
-    public function testSyncUser()
+    public function testSyncUsersContactData()
     {
-        $user = $this->testAddUser();
-        
-        Tinebase_User::syncUser($user, Tinebase_Application::getInstance()->isInstalled('Addressbook'));
+        Tinebase_Config::getInstance()->set(Tinebase_Config::LDAP_OVERWRITE_CONTACT_FIELDS, array(
+            'tel_work',
+            'jpegphoto'
+        ));
+
+        // add user in LDAP
+        $user = $this->_backend->addUserToSyncBackend(self::getTestRecord());
+        $this->_usernamesToDelete[] = $user->accountLoginName;
+
+        $syncedUser = Tinebase_User::syncUser($user);
+
+        // check if user is synced
+        $this->assertEquals(Tinebase_Model_User::VISIBILITY_DISPLAYED, $syncedUser->visibility,
+            print_r($syncedUser->toArray(), true));
+
+        // add+remove phone data in ldap and check if it is removed in adb, too
+        $syncOptions = array(
+            'syncContactData' => true,
+            'syncContactPhoto' => true,
+        );
+        $ldap = $this->_backend->getLdap();
+        $dn = $this->_backend->generateDn($syncedUser);
+        $number = '040-428457634';
+        $jpegImage = file_get_contents(dirname(dirname(dirname(dirname(__DIR__))))
+            . '/tine20/Admin/Setup/DemoData/persona_sclever.jpg');
+        $ldap->updateProperty($dn, array(
+            'telephonenumber' => $number,
+            'jpegphoto'       => $jpegImage,
+            //'birthdate'       => '2000-09-09'
+        ));
+        $syncedUser = Tinebase_User::syncUser($user, $syncOptions);
+        $contact = Addressbook_Controller_Contact::getInstance()->getContactByUserId($syncedUser->getId());
+        $this->assertEquals($number, $contact->tel_work);
+        $this->assertEquals(1, $contact->jpegphoto);
+        //$this->assertEquals('2000-09-09 00:00:00', $contact->bday->toString());
+
+        // remove number + photo and sync again
+        $ldap->updateProperty($dn, array(
+            'telephonenumber' => '',
+            'jpegphoto'       => '',
+        ));
+        $syncedUser = Tinebase_User::syncUser($user, $syncOptions);
+        $contact = Addressbook_Controller_Contact::getInstance()->getContactByUserId($syncedUser->getId());
+        $this->assertEquals('', $contact->tel_work);
+        $this->assertEquals(0, $contact->jpegphoto);
     }
-        
+
     /**
      * @return Tinebase_Model_FullUser
      */
index bb5bce0..f136565 100644 (file)
@@ -150,7 +150,14 @@ class Tinebase_Config extends Tinebase_Config_Abstract
      * @var string
      */
     const LDAP_DISABLE_TLSREQCERT = 'ldapDisableTlsReqCert';
-    
+
+    /**
+     * overwritten ldap fields
+     *
+     * @var string
+     */
+    const LDAP_OVERWRITE_CONTACT_FIELDS = 'ldapOverwriteContactFields';
+
     /**
      * configure hook class for user sync
      *
@@ -570,6 +577,19 @@ class Tinebase_Config extends Tinebase_Config_Abstract
             'setBySetupModule'      => true,
             'default'               => false
         ),
+        // TODO should this be added to LDAP config array/struct?
+        // TODO does this depend on LDAP readonly option?
+        self::LDAP_OVERWRITE_CONTACT_FIELDS => array(
+            //_('Contact fields overwritten by LDAP')
+            'label'                 => 'Contact fields overwritten by LDAP',
+            //_('These fields are overwritten during LDAP sync if empty')
+            'description'           => 'These fields are overwritten during LDAP sync if empty',
+            'type'                  => 'array',
+            'clientRegistryInclude' => false,
+            'setByAdminModule'      => false,
+            'setBySetupModule'      => true,
+            'default'               => array()
+        ),
         self::SYNC_USER_HOOK_CLASS => array(
                                    //_('Configure hook class for user sync')
             'label'                 => 'Configure hook class for user sync',
index 8f43621..dfeb698 100644 (file)
@@ -566,12 +566,12 @@ class Tinebase_User
             Tinebase_User::getInstance()->updateContactFromSyncBackend($syncedUser, $contact);
             $contact = self::_user2Contact($syncedUser, $contact);
 
-            if (Tinebase_Core::isLogLevel(Zend_Log::TRACE)) Tinebase_Core::getLogger()->trace(__METHOD__ . '::' . __LINE__
+            if (Tinebase_Core::isLogLevel(Zend_Log::DEBUG)) Tinebase_Core::getLogger()->debug(__METHOD__ . '::' . __LINE__
                 . ' new contact: ' . print_r($contact->toArray(), true)
                 . ' orig contact:' . print_r($originalContact->toArray(), true));
 
-            // TODO allow to diff jpegphoto, too / maybe this should only be done when called via CLI/cronjob
-            $diff = $contact->diff($originalContact, array('jpegphoto'));
+            $syncPhoto = isset($options['syncContactPhoto']) && $options['syncContactPhoto'];
+            $diff = $contact->diff($originalContact, $syncPhoto ? array() : array('jpegphoto'));
             if (! $diff->isEmpty() || ($originalContact->jpegphoto == 0 && ! empty($contact->jpegphoto))) {
                 // add modlog info
                 Tinebase_Timemachine_ModificationLog::setRecordMetaData($contact, 'update');
index 2ca44b1..3d19a1c 100644 (file)
@@ -149,7 +149,7 @@ class Tinebase_User_ActiveDirectory extends Tinebase_User_Ldap
             $plugin->inspectAddUser($_user, $ldapData);
         }
 
-        $dn = $this->_generateDn($_user);
+        $dn = $this->generateDn($_user);
         
         if (Tinebase_Core::isLogLevel(Zend_Log::TRACE)) 
             Tinebase_Core::getLogger()->trace(__METHOD__ . '::' . __LINE__ . '  ldapData: ' . print_r($ldapData, true));
@@ -343,7 +343,7 @@ class Tinebase_User_ActiveDirectory extends Tinebase_User_Ldap
         $dn = Zend_Ldap_Dn::factory($ldapEntry['dn'], null);
         $rdn = $dn->getRdn();
         if ($rdn['CN'] != $ldapData['cn']) {
-            $newDN = $this->_generateDn($_account);
+            $newDN = $this->generateDn($_account);
             if (Tinebase_Core::isLogLevel(Zend_Log::DEBUG))
                 Tinebase_Core::getLogger()->debug(__METHOD__ . '::' . __LINE__ . '  rename ldap entry to: ' . $newDN);
             $this->_ldap->rename($dn, $newDN);
@@ -419,7 +419,7 @@ class Tinebase_User_ActiveDirectory extends Tinebase_User_Ldap
      * @param  Tinebase_Model_FullUser $_account
      * @return string
      */
-    protected function _generateDn(Tinebase_Model_FullUser $_account)
+    public function generateDn(Tinebase_Model_FullUser $_account)
     {
         $newDn = "cn={$_account->accountFullName},{$this->_baseDn}";
 
index ada5c26..cfa6806 100644 (file)
@@ -543,7 +543,7 @@ class Tinebase_User_Ldap extends Tinebase_User_Sql implements Tinebase_User_Inte
                 $groupsBackend->removeGroupMemberInSyncBackend($groupId, $_account);
             }
             
-            $newDN = $this->_generateDn($_account);
+            $newDN = $this->generateDn($_account);
             if (Tinebase_Core::isLogLevel(Zend_Log::DEBUG)) Tinebase_Core::getLogger()->debug(__METHOD__ . '::' . __LINE__ . '  rename ldap entry to: ' . $newDN);
             $this->_ldap->rename($dn, $newDN);
             
@@ -580,7 +580,7 @@ class Tinebase_User_Ldap extends Tinebase_User_Sql implements Tinebase_User_Inte
             $plugin->inspectAddUser($user, $ldapData);
         }
 
-        $dn = $this->_generateDn($user);
+        $dn = $this->generateDn($user);
         
         if (Tinebase_Core::isLogLevel(Zend_Log::TRACE)) 
             Tinebase_Core::getLogger()->trace(__METHOD__ . '::' . __LINE__ . '  ldapData: ' . print_r($ldapData, true));
@@ -725,10 +725,9 @@ class Tinebase_User_Ldap extends Tinebase_User_Sql implements Tinebase_User_Inte
      * @param  Tinebase_Model_FullUser $_account
      * @return string
      */
-    protected function _generateDn(Tinebase_Model_FullUser $_account)
+    public function generateDn(Tinebase_Model_FullUser $_account)
     {
         $baseDn = $this->_baseDn;
-
         $uidProperty = array_search('uid', $this->_rowNameMapping);
         $newDn = "uid={$_account->$uidProperty},{$baseDn}";
 
@@ -1003,22 +1002,20 @@ class Tinebase_User_Ldap extends Tinebase_User_Sql implements Tinebase_User_Inte
             'adr_one_region'        => 'st',
         );
 
-        foreach ($_userData as $key => $value) {
-            if (is_int($key)) {
-                continue;
-            }
-
-            $keyMapping = array_search($key, $rowNameMapping);
-
-            if ($keyMapping !== FALSE) {
-                switch($keyMapping) {
+        $overwrittenFields = Tinebase_Config::getInstance()->get(Tinebase_Config::LDAP_OVERWRITE_CONTACT_FIELDS);
+        foreach ($rowNameMapping as $tineKey => $ldapKey) {
+            if (isset($_userData[$ldapKey])) {
+                switch ($tineKey) {
                     case 'bday':
-                        $_contact->$keyMapping = Tinebase_DateTime::createFromFormat('Y-m-d', $value[0]);
+                        $_contact->$tineKey = Tinebase_DateTime::createFromFormat('Y-m-d', $_userData[$ldapKey][0]);
                         break;
                     default:
-                        $_contact->$keyMapping = $value[0];
+                        $_contact->$tineKey = $_userData[$ldapKey][0];
                         break;
                 }
+            } else if (in_array($tineKey, $overwrittenFields)) {
+                // should empty values in ldap overwrite tine values
+                $_contact->$tineKey = '';
             }
         }
     }