From 88949d7f947b905415c19f1b61a0273fb27fa46b Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 20 Mar 2014 13:58:08 +0100 Subject: [PATCH] Backport of PR #7837 to stable6 LDAP Wizard: when determining objectclasses, we realy do not need to look at every entry. Fixes #7530 Conflicts: apps/user_ldap/lib/wizard.php Fix wildcard handling and check even less DNs per filter, enough will be looked at anyway Use the LDAP wrapper for checking resources, needs for proper testing fix potential infinite loop when the DN of the first entry was already read. add tests for cumulativeSearchOnAttribute make tests work on systems without php5_ldap define var add comment to clearify when a skip in the foreach happens --- apps/user_ldap/lib/wizard.php | 32 +++-- apps/user_ldap/tests/wizard.php | 210 ++++++++++++++++++++++++++++++++ 2 files changed, 235 insertions(+), 7 deletions(-) create mode 100644 apps/user_ldap/tests/wizard.php diff --git a/apps/user_ldap/lib/wizard.php b/apps/user_ldap/lib/wizard.php index 00623b74fb1..5d029a1f00a 100644 --- a/apps/user_ldap/lib/wizard.php +++ b/apps/user_ldap/lib/wizard.php @@ -869,7 +869,7 @@ class Wizard extends LDAPUtility { * @return mixed, an array with the values on success, false otherwise * */ - private function cumulativeSearchOnAttribute($filters, $attr, $lfw = true, &$maxF = null) { + public function cumulativeSearchOnAttribute($filters, $attr, $lfw = true, $dnReadLimit = 3, &$maxF = null) { $dnRead = array(); $foundItems = array(); $maxEntries = 0; @@ -879,11 +879,16 @@ class Wizard extends LDAPUtility { } $base = $this->configuration->ldapBase[0]; $cr = $this->getConnection(); - if(!is_resource($cr)) { + if(!$this->ldap->isResource($cr)) { return false; } + $lastFilter = null; + if(isset($filters[count($filters)-1])) { + $lastFilter = $filters[count($filters)-1]; + } foreach($filters as $filter) { - if($lfw && count($foundItems) > 0) { + if($lfw && $lastFilter === $filter && count($foundItems) > 0) { + //skip when the filter is a wildcard and results were found continue; } $rr = $this->ldap->search($cr, $base, $filter, array($attr)); @@ -897,8 +902,10 @@ class Wizard extends LDAPUtility { $maxEntries = $entries; $maxF = $filter; } + $dnReadCount = 0; do { $entry = $this->ldap->$getEntryFunc($cr, $rr); + $getEntryFunc = 'nextEntry'; if(!$this->ldap->isResource($entry)) { continue 2; } @@ -911,13 +918,14 @@ class Wizard extends LDAPUtility { $state = $this->getAttributeValuesFromEntry($attributes, $attr, $newItems); + $dnReadCount++; $foundItems = array_merge($foundItems, $newItems); $this->resultCache[$dn][$attr] = $newItems; $dnRead[] = $dn; - $getEntryFunc = 'nextEntry'; $rr = $entry; //will be expected by nextEntry next round - } while($state === self::LRESULT_PROCESSED_SKIP - || $this->ldap->isResource($entry)); + } while(($state === self::LRESULT_PROCESSED_SKIP + || $this->ldap->isResource($entry)) + && ($dnReadLimit === 0 || $dnReadCount < $dnReadLimit)); } } @@ -945,9 +953,19 @@ class Wizard extends LDAPUtility { $objectclasses[$key] = $p.$value; } $maxEntryObjC = ''; + + //how deep to dig? + //When looking for objectclasses, testing few entries is sufficient, + //when looking for group we need to get all names, though. + if(strtolower($attr) === 'objectclass') { + $dig = 3; + } else { + $dig = 0; + } + $availableFeatures = $this->cumulativeSearchOnAttribute($objectclasses, $attr, - true, $maxEntryObjC); + true, $dig, $maxEntryObjC); if(is_array($availableFeatures) && count($availableFeatures) > 0) { natcasesort($availableFeatures); diff --git a/apps/user_ldap/tests/wizard.php b/apps/user_ldap/tests/wizard.php new file mode 100644 index 00000000000..2b5cabc705d --- /dev/null +++ b/apps/user_ldap/tests/wizard.php @@ -0,0 +1,210 @@ +. +* +*/ + +namespace OCA\user_ldap\tests; + +use \OCA\user_ldap\lib\Wizard; + +// use \OCA\user_ldap\USER_LDAP as UserLDAP; +// use \OCA\user_ldap\lib\Access; +// use \OCA\user_ldap\lib\Configuration; +// use \OCA\user_ldap\lib\ILDAPWrapper; + +class Test_Wizard extends \PHPUnit_Framework_TestCase { + public function setUp() { + //we need to make sure the consts are defined, otherwise tests will fail + //on systems without php5_ldap + $ldapConsts = array('LDAP_OPT_PROTOCOL_VERSION', + 'LDAP_OPT_REFERRALS', 'LDAP_OPT_NETWORK_TIMEOUT'); + foreach($ldapConsts as $const) { + if(!defined($const)) { + define($const, 42); + } + } + } + + private function getWizardAndMocks() { + static $conMethods; + + if(is_null($conMethods)) { + $conMethods = get_class_methods('\OCA\user_ldap\lib\Configuration'); + } + $lw = $this->getMock('\OCA\user_ldap\lib\ILDAPWrapper'); + $conf = $this->getMock('\OCA\user_ldap\lib\Configuration', + $conMethods, + array($lw, null, null)); + return array(new Wizard($conf, $lw), $conf, $lw); + } + + private function prepareLdapWrapperForConnections(&$ldap) { + $ldap->expects($this->once()) + ->method('connect') + //dummy value, usually invalid + ->will($this->returnValue(true)); + + $ldap->expects($this->exactly(3)) + ->method('setOption') + ->will($this->returnValue(true)); + + $ldap->expects($this->once()) + ->method('bind') + ->will($this->returnValue(true)); + + } + + public function testCumulativeSearchOnAttributeLimited() { + list($wizard, $configuration, $ldap) = $this->getWizardAndMocks(); + + $configuration->expects($this->any()) + ->method('__get') + ->will($this->returnCallback(function($name) { + if($name === 'ldapBase') { + return array('base'); + } + return null; + })); + + $this->prepareLdapWrapperForConnections($ldap); + + $ldap->expects($this->any()) + ->method('isResource') + ->will($this->returnValue(true)); + + $ldap->expects($this->exactly(2)) + ->method('search') + //dummy value, usually invalid + ->will($this->returnValue(true)); + + $ldap->expects($this->exactly(2)) + ->method('countEntries') + //an is_resource check will follow, so we need to return a dummy resource + ->will($this->returnValue(23)); + + //5 DNs per filter means 2x firstEntry and 8x nextEntry + $ldap->expects($this->exactly(2)) + ->method('firstEntry') + //dummy value, usually invalid + ->will($this->returnValue(true)); + + $ldap->expects($this->exactly(8)) + ->method('nextEntry') + //dummy value, usually invalid + ->will($this->returnValue(true)); + + $ldap->expects($this->exactly(10)) + ->method('getAttributes') + //dummy value, usually invalid + ->will($this->returnValue(array('cn' => array('foo'), 'count' => 1))); + + global $uidnumber; + $uidnumber = 1; + $ldap->expects($this->exactly(10)) + ->method('getDN') + //dummy value, usually invalid + ->will($this->returnCallback(function($a, $b) { + global $uidnumber; + return $uidnumber++; + })); + + # The following expectations are the real test # + $filters = array('f1', 'f2', '*'); + $wizard->cumulativeSearchOnAttribute($filters, 'cn', true, 5); + unset($uidnumber); + } + + public function testCumulativeSearchOnAttributeUnlimited() { + list($wizard, $configuration, $ldap) = $this->getWizardAndMocks(); + + $configuration->expects($this->any()) + ->method('__get') + ->will($this->returnCallback(function($name) { + if($name === 'ldapBase') { + return array('base'); + } + return null; + })); + + $this->prepareLdapWrapperForConnections($ldap); + + $ldap->expects($this->any()) + ->method('isResource') + ->will($this->returnCallback(function($r) { + if($r === true) { + return true; + } + if($r % 24 === 0) { + global $uidnumber; + $uidnumber++; + return false; + } + return true; + })); + + $ldap->expects($this->exactly(2)) + ->method('search') + //dummy value, usually invalid + ->will($this->returnValue(true)); + + $ldap->expects($this->exactly(2)) + ->method('countEntries') + //an is_resource check will follow, so we need to return a dummy resource + ->will($this->returnValue(23)); + + //5 DNs per filter means 2x firstEntry and 8x nextEntry + $ldap->expects($this->exactly(2)) + ->method('firstEntry') + //dummy value, usually invalid + ->will($this->returnCallback(function($r) { + global $uidnumber; + return $uidnumber; + })); + + $ldap->expects($this->exactly(46)) + ->method('nextEntry') + //dummy value, usually invalid + ->will($this->returnCallback(function($r) { + global $uidnumber; + return $uidnumber; + })); + + $ldap->expects($this->exactly(46)) + ->method('getAttributes') + //dummy value, usually invalid + ->will($this->returnValue(array('cn' => array('foo'), 'count' => 1))); + + global $uidnumber; + $uidnumber = 1; + $ldap->expects($this->exactly(46)) + ->method('getDN') + //dummy value, usually invalid + ->will($this->returnCallback(function($a, $b) { + global $uidnumber; + return $uidnumber++; + })); + + # The following expectations are the real test # + $filters = array('f1', 'f2', '*'); + $wizard->cumulativeSearchOnAttribute($filters, 'cn', true, 0); + unset($uidnumber); + } + +} \ No newline at end of file