Merge pull request #27906 from owncloud/account-terms-table

User backend extendable account searching

apps/files_sharing/lib/Controller/ShareesController.php

@@ -3,8 +3,8 @@
  * @author Björn Schießle <bjoern@schiessle.org>
  * @author Joas Schilling <coding@schilljs.com>
  * @author Roeland Jago Douma <rullzer@owncloud.com>
- * @author Roeland Jago Douma <rullzer@users.noreply.github.com>
  * @author Thomas Müller <thomas.mueller@tmit.eu>
+ * @author Tom Needham <tom@owncloud.com>
  * @author Vincent Petry <pvince81@owncloud.com>
  *
  * @copyright Copyright (c) 2017, ownCloud GmbH
@@ -144,17 +144,17 @@ protected function getUsers($search) {
 			// Search in all the groups this user is part of
 			$userGroups = $this->groupManager->getUserGroupIds($this->userSession->getUser());
 			foreach ($userGroups as $userGroup) {
-				$usersTmp = $this->groupManager->displayNamesInGroup($userGroup, $search, $this->limit, $this->offset);
-				foreach ($usersTmp as $uid => $userDisplayName) {
-					$users[$uid] = $userDisplayName;
+				$usersTmp = $this->groupManager->findUsersInGroup($userGroup, $search, $this->limit, $this->offset);
+				foreach ($usersTmp as $uid => $user) {
+					$users[$uid] = $user;
 				}
 			}
 		} else {
 			// Search in all users
-			$usersTmp = $this->userManager->searchDisplayName($search, $this->limit, $this->offset);
+			$usersTmp = $this->userManager->find($search, $this->limit, $this->offset);
 
 			foreach ($usersTmp as $user) {
-				$users[$user->getUID()] = $user->getDisplayName();
+				$users[$user->getUID()] = $user;
 			}
 		}
 
@@ -164,21 +164,22 @@ protected function getUsers($search) {
 
 		$foundUserById = false;
 		$lowerSearch = strtolower($search);
-		foreach ($users as $uid => $userDisplayName) {
-			if (strtolower($uid) === $lowerSearch || strtolower($userDisplayName) === $lowerSearch) {
+		foreach ($users as $uid => $user) {
+			/* @var $user IUser */
+			if (strtolower($uid) === $lowerSearch || strtolower($user->getDisplayName()) === $lowerSearch || strtolower($user->getEMailAddress()) === $lowerSearch) {
 				if (strtolower($uid) === $lowerSearch) {
 					$foundUserById = true;
 				}
 				$this->result['exact']['users'][] = [
-					'label' => $userDisplayName,
+					'label' => $user->getDisplayName(),
 					'value' => [
 						'shareType' => Share::SHARE_TYPE_USER,
 						'shareWith' => $uid,
 					],
 				];
 			} else {
 				$this->result['users'][] = [
-					'label' => $userDisplayName,
+					'label' => $user->getDisplayName(),
 					'value' => [
 						'shareType' => Share::SHARE_TYPE_USER,
 						'shareWith' => $uid,

apps/files_sharing/tests/API/ShareesTest.php

@@ -3,8 +3,8 @@
  * @author Björn Schießle <bjoern@schiessle.org>
  * @author Joas Schilling <coding@schilljs.com>
  * @author Roeland Jago Douma <rullzer@owncloud.com>
- * @author Roeland Jago Douma <rullzer@users.noreply.github.com>
  * @author Thomas Müller <thomas.mueller@tmit.eu>
+ * @author Tom Needham <tom@owncloud.com>
  * @author Vincent Petry <pvince81@owncloud.com>
  *
  * @copyright Copyright (c) 2017, ownCloud GmbH
@@ -304,7 +304,7 @@ public function dataGetUsers() {
 				true,
 				['abc', 'xyz'],
 				[
-					['abc', 'test', 2, 0, ['test1' => 'Test One']],
+					['abc', 'test', 2, 0, ['test1' => $this->getUserMock('test1', 'Test One')]],
 					['xyz', 'test', 2, 0, []],
 				],
 				[],
@@ -320,7 +320,7 @@ public function dataGetUsers() {
 				false,
 				['abc', 'xyz'],
 				[
-					['abc', 'test', 2, 0, ['test1' => 'Test One']],
+					['abc', 'test', 2, 0, ['test1' => $this->getUserMock('test1', 'Test One')]],
 					['xyz', 'test', 2, 0, []],
 				],
 				[],
@@ -335,12 +335,12 @@ public function dataGetUsers() {
 				['abc', 'xyz'],
 				[
 					['abc', 'test', 2, 0, [
-						'test1' => 'Test One',
-						'test2' => 'Test Two',
+						'test1' => $this->getUserMock('test1', 'Test One'),
+						'test2' => $this->getUserMock('test2', 'Test Two'),
 					]],
 					['xyz', 'test', 2, 0, [
-						'test1' => 'Test One',
-						'test2' => 'Test Two',
+						'test1' => $this->getUserMock('test1', 'Test One'),
+						'test2' => $this->getUserMock('test2', 'Test Two'),
 					]],
 				],
 				[],
@@ -358,12 +358,12 @@ public function dataGetUsers() {
 				['abc', 'xyz'],
 				[
 					['abc', 'test', 2, 0, [
-						'test1' => 'Test One',
-						'test2' => 'Test Two',
+						'test1' => $this->getUserMock('test1', 'Test One'),
+						'test2' => $this->getUserMock('test2', 'Test Two'),
 					]],
 					['xyz', 'test', 2, 0, [
-						'test1' => 'Test One',
-						'test2' => 'Test Two',
+						'test1' => $this->getUserMock('test1', 'Test One'),
+						'test2' => $this->getUserMock('test2', 'Test Two'),
 					]],
 				],
 				[],
@@ -378,10 +378,10 @@ public function dataGetUsers() {
 				['abc', 'xyz'],
 				[
 					['abc', 'test', 2, 0, [
-						'test' => 'Test One',
+						'test' => $this->getUserMock('test1', 'Test One'),
 					]],
 					['xyz', 'test', 2, 0, [
-						'test2' => 'Test Two',
+						'test2' => $this->getUserMock('test2', 'Test Two'),
 					]],
 				],
 				[
@@ -400,10 +400,10 @@ public function dataGetUsers() {
 				['abc', 'xyz'],
 				[
 					['abc', 'test', 2, 0, [
-						'test' => 'Test One',
+						'test' => $this->getUserMock('test1', 'Test One'),
 					]],
 					['xyz', 'test', 2, 0, [
-						'test2' => 'Test Two',
+						'test2' => $this->getUserMock('test2', 'Test Two'),
 					]],
 				],
 				[
@@ -424,7 +424,7 @@ public function dataGetUsers() {
 				// args and user response for "displayNamesInGroup" call
 				[
 					['group1', 'ano', 2, 0, [
-						'another1' => 'Another One',
+						'another1' => $this->getUserMock('another1', 'Another One'),
 					]],
 					['group2', 'ano', 2, 0, [
 					]],
@@ -504,7 +504,7 @@ public function testGetUsers(
 
 		if (!$shareWithGroupOnly && !$shareeEnumerationGroupMembers) {
 			$this->userManager->expects($this->once())
-				->method('searchDisplayName')
+				->method('find')
 				->with($searchTerm, $this->invokePrivate($this->sharees, 'limit'), $this->invokePrivate($this->sharees, 'offset'))
 				->willReturn($userResponse);
 		} else {
@@ -527,7 +527,7 @@ public function testGetUsers(
 			}
 
 			$this->groupManager->expects($this->exactly(sizeof($groupResponse)))
-				->method('displayNamesInGroup')
+				->method('findUsersInGroup')
 				->with($this->anything(), $searchTerm, $this->invokePrivate($this->sharees, 'limit'), $this->invokePrivate($this->sharees, 'offset'))
 				->willReturnMap($userResponse);
 		}

config/config.sample.php

@@ -256,6 +256,13 @@
  */
 'lost_password_link' => 'https://example.org/link/to/password/reset',
 
+/**
+ * Allow medial search on account properties like display name, user id, email,
+ * and other search terms. Allows finding 'Alice' when searching for 'lic'.
+ * May slow down user search.
+ */
+'accounts.enable_medial_search' => false,
+
 /**
  * Mail Parameters
  *

core/Migrations/Version20170221114437.php

@@ -3,9 +3,9 @@
 
 use OC\User\Account;
 use OC\User\AccountMapper;
+use OC\User\AccountTermMapper;
 use OC\User\Database;
 use OC\User\SyncService;
-use OCP\IConfig;
 use OCP\Migration\ISimpleMigration;
 use OCP\Migration\IOutput;
 
@@ -16,9 +16,10 @@ class Version20170221114437 implements ISimpleMigration {
 	 */
 	public function run(IOutput $out) {
 		$backend = new Database();
-		$accountMapper = new AccountMapper(\OC::$server->getDatabaseConnection());
 		$config = \OC::$server->getConfig();
 		$logger = \OC::$server->getLogger();
+		$connection = \OC::$server->getDatabaseConnection();
+		$accountMapper = new AccountMapper($connection, new AccountTermMapper($connection));
 		$syncService = new SyncService($accountMapper, $backend, $config, $logger);
 
 		// insert/update known users

core/Migrations/Version20170516100103.php

@@ -0,0 +1,75 @@
+<?php
+/**
+ * @author Jörn Friedrich Dreyer <jfd@butonic.de>
+ * @author Tom Needham <tom@owncloud.com>
+ *
+ * @copyright Copyright (c) 2017, ownCloud GmbH
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program 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, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace OC\Migrations;
+
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\DBAL\Types\Type;
+use OCP\Migration\ISchemaMigration;
+
+/**
+ * Add account_terms table for account searching
+ */
+class Version20170516100103 implements ISchemaMigration {
+
+	public function changeSchema(Schema $schema, array $options) {
+		$prefix = $options['tablePrefix'];
+		if (!$schema->hasTable("${prefix}account_terms")) {
+			$table = $schema->createTable("${prefix}account_terms");
+
+			$table->addColumn('id', Type::BIGINT, [
+				'autoincrement' => true,
+				'unsigned' => true,
+				'notnull' => true,
+			]);
+
+			// Foreign key to oc_accounts.id column
+			$table->addColumn('account_id', Type::BIGINT, [
+				'notnull' => true,
+				'unsigned' => true,
+			]);
+
+			$table->addColumn('term', Type::STRING, [
+				'notnull' => true,
+				'length' => 256
+			]);
+
+			$table->setPrimaryKey(['id']);
+			$table->addIndex(['account_id'], 'account_id_index');
+			$table->addIndex(['term'], 'term_index');
+
+		}
+
+		if ($schema->hasTable("${prefix}accounts")) {
+			$table = $schema->getTable("${prefix}accounts");
+			if (!$table->hasIndex('lower_user_id_index')) {
+				$table->addUniqueIndex(['lower_user_id'], 'lower_user_id_index');
+			}
+			if (!$table->hasIndex('display_name_index')) {
+				$table->addIndex(['display_name'], 'display_name_index');
+			}
+			if (!$table->hasIndex('email_index')) {
+				$table->addIndex(['email'], 'email_index');
+			}
+		}
+    }
+}

core/register_command.php

@@ -143,7 +143,7 @@
 	$application->add(new OC\Core\Command\User\Report(\OC::$server->getUserManager()));
 	$application->add(new OC\Core\Command\User\ResetPassword(\OC::$server->getUserManager()));
 	$application->add(new OC\Core\Command\User\Setting(\OC::$server->getUserManager(), \OC::$server->getConfig(), \OC::$server->getDatabaseConnection()));
-	$application->add(new OC\Core\Command\User\SyncBackend(new \OC\User\AccountMapper(\OC::$server->getDatabaseConnection()), \OC::$server->getConfig(), \OC::$server->getUserManager(), \OC::$server->getLogger()));
+	$application->add(new OC\Core\Command\User\SyncBackend(\OC::$server->getAccountMapper(), \OC::$server->getConfig(), \OC::$server->getUserManager(), \OC::$server->getLogger()));
 
 	$application->add(new OC\Core\Command\Security\ListCertificates(\OC::$server->getCertificateManager(null), \OC::$server->getL10N('core')));
 	$application->add(new OC\Core\Command\Security\ImportCertificate(\OC::$server->getCertificateManager(null)));

lib/private/Group/Manager.php

@@ -349,6 +349,58 @@ public function getUserGroupIds($user, $scope = null) {
 		}, array_keys($this->getUserGroups($user, $scope)));
 	}
 
+	/**
+	 * Finds users in a group
+	 * @param string $gid
+	 * @param string $search
+	 * @param int $limit
+	 * @param int $offset
+	 * @return \OC\User\User[]
+	 */
+	public function findUsersInGroup($gid, $search = '', $limit = -1, $offset = 0) {
+		$group = $this->get($gid);
+		if(is_null($group)) {
+			return [];
+		}
+
+		$search = trim($search);
+		$groupUsers = [];
+
+		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 = 500;
+			}
+
+			do {
+				$filteredUsers = $this->userManager->find($search, $searchLimit, $searchOffset);
+				foreach($filteredUsers as $filteredUser) {
+					if($group->inGroup($filteredUser)) {
+						$groupUsers[]= $filteredUser;
+					}
+				}
+				$searchOffset += $searchLimit;
+			} while(count($groupUsers) < $searchLimit+$offset && count($filteredUsers) >= $searchLimit);
+
+			if($limit === -1) {
+				$groupUsers = array_slice($groupUsers, $offset);
+			} else {
+				$groupUsers = array_slice($groupUsers, $offset, $limit);
+			}
+		} else {
+			$groupUsers = $group->searchUsers('', $limit, $offset);
+		}
+
+		$matchingUsers = [];
+		foreach($groupUsers as $groupUser) {
+			$matchingUsers[$groupUser->getUID()] = $groupUser;
+		}
+
+		return $matchingUsers;
+	}
+
 	/**
 	 * get a list of all display names in a group
 	 * @param string $gid

lib/private/Server.php

@@ -86,6 +86,7 @@
 use OC\Tagging\TagMapper;
 use OC\Theme\ThemeService;
 use OC\User\AccountMapper;
+use OC\User\AccountTermMapper;
 use OCP\IL10N;
 use OCP\IServerContainer;
 use OCP\ISession;
@@ -220,11 +221,13 @@ public function __construct($webRoot, \OC\Config $config) {
 				return $c->getRootFolder();
 			});
 		});
+		$this->registerService('AccountMapper', function(Server $c) {
+			return new AccountMapper($c->getDatabaseConnection(), new AccountTermMapper($c->getDatabaseConnection()));
+		});
 		$this->registerService('UserManager', function (Server $c) {
 			$config = $c->getConfig();
 			$logger = $c->getLogger();
-			$accountMapper = new AccountMapper($c->getDatabaseConnection());
-			return new \OC\User\Manager($config, $logger, $accountMapper);
+			return new \OC\User\Manager($config, $logger, $c->getAccountMapper());
 		});
 		$this->registerService('GroupManager', function (Server $c) {
 			$groupManager = new \OC\Group\Manager($this->getUserManager());
@@ -953,6 +956,13 @@ public function getUserManager() {
 		return $this->query('UserManager');
 	}
 
+	/**
+	 * @return \OC\User\AccountMapper
+	 */
+	public function getAccountMapper() {
+		return $this->query('AccountMapper');
+	}
+
 	/**
 	 * @return \OC\Group\Manager
 	 */