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

2 files changed, 235 insertions, 7 deletions

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 @@
+<?php
+/**
+* ownCloud
+*
+* @author Arthur Schiwon
+* @copyright 2014 Arthur Schiwon blizzz@owncloud.com
+*
+* This library is free software; you can redistribute it and/or
+* modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
+* License as published by the Free Software Foundation; either
+* version 3 of the License, or any later version.
+*
+* This library is distributed in the hope that it will be useful,
+* but WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+* GNU AFFERO GENERAL PUBLIC LICENSE for more details.
+*
+* You should have received a copy of the GNU Affero General Public
+* License along with this library.  If not, see <http://www.gnu.org/licenses/>.
+*
+*/
+
+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