backport of #11494

fix retrievel of group members and cache group members

fix changed variable name

with several backends, more than limit can be returned

make performance less bad. Still far from good, but at least it works

add one simple cache test

adjust group manager tests

Conflicts:
	apps/user_ldap/group_ldap.php
	apps/user_ldap/lib/access.php
	apps/user_ldap/tests/group_ldap.php

4 files changed, 65 insertions, 34 deletions

diff --git a/apps/user_ldap/group_ldap.php b/apps/user_ldap/group_ldap.php
index 30217412611..dafb29e901c 100644
--- a/apps/user_ldap/group_ldap.php
+++ b/apps/user_ldap/group_ldap.php
@@ -29,6 +29,11 @@ use OCA\user_ldap\lib\BackendUtility;
 class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface {
 	protected $enabled = false;
 
+	/**
+	 * @var string[] $cachedGroupMembers array of users with gid as key
+	 */
+	protected $cachedGroupMembers = array();
+
 	public function __construct(Access $access) {
 		parent::__construct($access);
 		$filter = $this->access->connection->ldapGroupFilter;
@@ -55,6 +60,21 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface {
 		}
 		$dn_user = $this->access->username2dn($uid);
 		$dn_group = $this->access->groupname2dn($gid);
+
+		if(isset($this->cachedGroupMembers[$gid])) {
+			$isInGroup = in_array($dn_user, $this->cachedGroupMembers[$gid]);
+			return $isInGroup;
+		}
+
+		$cacheKeyMembers = 'inGroup-members:'.$gid;
+		if($this->access->connection->isCached($cacheKeyMembers)) {
+			$members = $this->access->connection->getFromCache($cacheKeyMembers);
+			$this->cachedGroupMembers[$gid] = $members;
+			$isInGroup = in_array($dn_user, $members);
+			$this->access->connection->writeToCache($cacheKey, $isInGroup);
+			return $isInGroup;
+		}
+
 		// just in case
 		if(!$dn_group || !$dn_user) {
 			$this->access->connection->writeToCache('inGroup'.$uid.':'.$gid, false);
@@ -62,29 +82,42 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface {
 		}
 		//usually, LDAP attributes are said to be case insensitive. But there are exceptions of course.
 		$members = $this->access->readAttribute($dn_group,
-						$this->access->connection->ldapGroupMemberAssocAttr);
-		if(!$members) {
+			$this->access->connection->ldapGroupMemberAssocAttr);
+		if(!is_array($members) || count($members) === 0) {
 			$this->access->connection->writeToCache('inGroup'.$uid.':'.$gid, false);
 			return false;
 		}
 
 		//extra work if we don't get back user DNs
-		//TODO: this can be done with one LDAP query
 		if(strtolower($this->access->connection->ldapGroupMemberAssocAttr) === 'memberuid') {
 			$dns = array();
+			$filterParts = array();
+			$bytes = 0;
 			foreach($members as $mid) {
 				$filter = str_replace('%uid', $mid, $this->access->connection->ldapLoginFilter);
-				$ldap_users = $this->access->fetchListOfUsers($filter, 'dn');
-				if(count($ldap_users) < 1) {
-					continue;
+				$filterParts[] = $filter;
+				$bytes += strlen($filter);
+				if($bytes >= 9000000) {
+					// AD has a default input buffer of 10 MB, we do not want
+					// to take even the chance to exceed it
+					$filter = $this->access->combineFilterWithOr($filterParts);
+					$bytes = 0;
+					$filterParts = array();
+					$users = $this->access->fetchListOfUsers($filter, 'dn', count($filterParts));
+					$dns = array_merge($dns, $users);
 				}
-				$dns[] = $ldap_users[0];
+			}
+			if(count($filterParts) > 0) {
+				$filter = $this->access->combineFilterWithOr($filterParts);
+				$users = $this->access->fetchListOfUsers($filter, 'dn', count($filterParts));
+				$dns = array_merge($dns, $users);
 			}
 			$members = $dns;
 		}
-
 		$isInGroup = in_array($dn_user, $members);
 		$this->access->connection->writeToCache('inGroup'.$uid.':'.$gid, $isInGroup);
+		$this->access->connection->writeToCache($cacheKeyMembers, $members);
+		$this->cachedGroupMembers[$gid] = $members;
 
 		return $isInGroup;
 	}
@@ -123,13 +156,18 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface {
 			// just in case
 			$uid = $userDN;
 		}
+				
+		if(isset($this->cachedGroupsByMember[$uid])) {
+			$groups = $this->cachedGroupsByMember[$uid];
+		} else {
+			$filter = $this->access->combineFilterWithAnd(array(
+				$this->access->connection->ldapGroupFilter,
+				$this->access->connection->ldapGroupMemberAssocAttr.'='.$uid
+			));
+			$groups = $this->access->fetchListOfGroups($filter,
+					array($this->access->connection->ldapGroupDisplayName, 'dn'));
+		}
 
-		$filter = $this->access->combineFilterWithAnd(array(
-			$this->access->connection->ldapGroupFilter,
-			$this->access->connection->ldapGroupMemberAssocAttr.'='.$uid
-		));
-		$groups = $this->access->fetchListOfGroups($filter,
-				array($this->access->connection->ldapGroupDisplayName, 'dn'));
 		$groups = array_unique($this->access->ownCloudGroupNames($groups), SORT_LOCALE_STRING);
 		$this->access->connection->writeToCache($cacheKey, $groups);
 
diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php
index 06d700afb02..8bde04faed3 100644
--- a/apps/user_ldap/lib/access.php
+++ b/apps/user_ldap/lib/access.php
@@ -1250,7 +1250,7 @@ class Access extends LDAPUtility {
 	 * @param $bases array containing the allowed base DN or DNs
 	 * @returns Boolean
 	 */
-	private function isDNPartOfBase($dn, $bases) {
+	public function isDNPartOfBase($dn, $bases) {
 		$bases = $this->sanitizeDN($bases);
 		foreach($bases as $base) {
 			$belongsToBase = true;
diff --git a/lib/private/group/manager.php b/lib/private/group/manager.php
index 2a2fa5a67f7..54348c8f122 100644
--- a/lib/private/group/manager.php
+++ b/lib/private/group/manager.php
@@ -179,10 +179,9 @@ class Manager extends PublicEmitter {
 		if(!empty($search)) {
 			// only user backends have the capability to do a complex search for users
 			$searchOffset = 0;
+			$searchLimit = $limit * 100;
 			if($limit === -1) {
-				$searchLimit = $group->count('');
-			} else {
-				$searchLimit = $limit * 2;
+				$searchLimit = 500;
 			}
 
 			do {
@@ -193,7 +192,7 @@ class Manager extends PublicEmitter {
 					}
 				}
 				$searchOffset += $searchLimit;
-			} while(count($groupUsers) < $searchLimit+$offset && count($filteredUsers) === $searchLimit);
+			} while(count($groupUsers) < $searchLimit+$offset && count($filteredUsers) >= $searchLimit);
 
 			if($limit === -1) {
 				$groupUsers = array_slice($groupUsers, $offset);
diff --git a/tests/lib/group/manager.php b/tests/lib/group/manager.php
index 54e8fc4f893..8ab35599007 100644
--- a/tests/lib/group/manager.php
+++ b/tests/lib/group/manager.php
@@ -367,15 +367,6 @@ class Manager extends \PHPUnit_Framework_TestCase {
                                 }
                         }));
 
-		$backend->expects($this->once())
-			->method('implementsActions')
-			->will($this->returnValue(true));
-
-		$backend->expects($this->once())
-			->method('countUsersInGroup')
-			->with('testgroup', '')
-			->will($this->returnValue(2));
-
 		/**
 		 * @var \OC\User\Manager $userManager
 		 */
@@ -494,9 +485,9 @@ class Manager extends \PHPUnit_Framework_TestCase {
 			->with('testgroup')
 			->will($this->returnValue(true));
 
-                $backend->expects($this->any())
-			->method('InGroup')
-			->will($this->returnCallback(function($uid, $gid) {
+        $backend->expects($this->any())
+			->method('inGroup')
+			->will($this->returnCallback(function($uid) {
                                 switch($uid) {
                                         case 'user1' : return false;
                                         case 'user2' : return true;
@@ -519,9 +510,12 @@ class Manager extends \PHPUnit_Framework_TestCase {
 			->with('user3')
 			->will($this->returnCallback(function($search, $limit, $offset) use ($userBackend) {
                                 switch($offset) {
-                                        case 0 : return array('user3' => new User('user3', $userBackend),
-                                                        'user33' => new User('user33', $userBackend));
-                                        case 2 : return array('user333' => new User('user333', $userBackend));
+                                        case 0 :
+											return array(
+												'user3' => new User('user3', $userBackend),
+                                                'user33' => new User('user33', $userBackend),
+												'user333' => new User('user333', $userBackend)
+											);
                                 }
                         }));