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>
Fri, 19 Aug 2016 07:26:32 +0000 (09:26 +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/3448
Tested-by: Jenkins CI (http://ci.tine20.com/)
Reviewed-by: Philipp Schüle <p.schuele@metaways.de>
tests/tine20/Tinebase/User/LdapTest.php
tine20/Setup/Frontend/Cli.php
tine20/Setup/Server/Cli.php
tine20/Tinebase/Config.php
tine20/Tinebase/User.php
tine20/Tinebase/User/ActiveDirectory.php
tine20/Tinebase/User/Ldap.php

index 2d8d2e6..206745e 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
      * 
@@ -215,14 +222,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 353e55e..51c06f9 100644 (file)
@@ -314,6 +314,10 @@ class Setup_Frontend_Cli
             $options['deleteUsers'] = true;
         }
 
+        if ($_opts->syncontactphoto) {
+            $options['syncContactPhoto'] = true;
+        }
+
         Tinebase_User::syncUsers($options);
     }
     
index 1d5af2f..0747ac7 100644 (file)
@@ -49,6 +49,7 @@ class Setup_Server_Cli implements Tinebase_Server_Interface
                     'dbmailldap'            => 'Only usable with sync_accounts_from_ldap. Fetches dbmail email user data from LDAP.',
                     'onlyusers'             => 'Only usable with sync_accounts_from_ldap. Fetches only users and no groups from LDAP.',
                     'syncdeletedusers'      => 'Only usable with sync_accounts_from_ldap. Removes users from Tine 2.0 DB that no longer exist in LDAP',
+                    'syncontactphoto'      => 'Only usable with sync_accounts_from_ldap. Always syncs contact photo from ldap',
                 'sync_passwords_from_ldap'  => 'Synchronize user passwords from ldap',
                 'egw14import'               => 'Import user and groups from egw14
                          Examples: 
index 4ad1b72..a72c9da 100644 (file)
@@ -136,7 +136,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
      *
@@ -471,6 +478,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 8f3cde7..cc37581 100644 (file)
@@ -548,12 +548,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 0f43494..57a1b3d 100644 (file)
@@ -148,7 +148,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));
@@ -352,7 +352,7 @@ class Tinebase_User_ActiveDirectory extends Tinebase_User_Ldap
         
         // do we need to rename the entry?
         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);
@@ -414,7 +414,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 560d75d..da611e5 100644 (file)
@@ -502,7 +502,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);
             
@@ -539,7 +539,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));
@@ -685,10 +685,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}";
 
@@ -923,22 +922,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)->toArray();
+        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 = '';
             }
         }
     }