Merge pull request #24659 from nextcloud/enh/noid/dav-honour-sharing.maxAutocompleteResults

dav principal search to honour sharing.maxAutocompleteResults setting

apps/dav/lib/Connector/Sabre/Principal.php

@@ -39,6 +39,7 @@ use OCA\DAV\CalDAV\Proxy\ProxyMapper;
 use OCA\DAV\Traits\PrincipalProxyTrait;
 use OCP\App\IAppManager;
 use OCP\AppFramework\QueryException;
+use OCP\Constants;
 use OCP\IConfig;
 use OCP\IGroupManager;
 use OCP\IUser;
@@ -270,6 +271,10 @@ class Principal implements BackendInterface {
 			}
 		}
 
+		$searchLimit = $this->config->getSystemValueInt('sharing.maxAutocompleteResults', Constants::SHARING_MAX_AUTOCOMPLETE_RESULTS_DEFAULT);
+		if ($searchLimit <= 0) {
+			$searchLimit = null;
+		}
 		foreach ($searchProperties as $prop => $value) {
 			switch ($prop) {
 				case '{http://sabredav.org/ns}email-address':
@@ -305,7 +310,7 @@ class Principal implements BackendInterface {
 					break;
 
 				case '{DAV:}displayname':
-					$users = $this->userManager->searchDisplayName($value);
+					$users = $this->userManager->searchDisplayName($value, $searchLimit);
 
 					if (!$allowEnumeration) {
 						$users = \array_filter($users, static function (IUser $user) use ($value) {

apps/dav/lib/DAV/GroupPrincipalBackend.php

@@ -27,6 +27,8 @@
 
 namespace OCA\DAV\DAV;
 
+use OCP\Constants;
+use OCP\IConfig;
 use OCP\IGroup;
 use OCP\IGroupManager;
 use OCP\IUser;
@@ -47,18 +49,24 @@ class GroupPrincipalBackend implements BackendInterface {
 
 	/** @var IShareManager */
 	private $shareManager;
+	/** @var IConfig */
+	private $config;
 
 	/**
 	 * @param IGroupManager $IGroupManager
 	 * @param IUserSession $userSession
 	 * @param IShareManager $shareManager
 	 */
-	public function __construct(IGroupManager $IGroupManager,
+	public function __construct(
-								IUserSession $userSession,
+		IGroupManager $IGroupManager,
-								IShareManager $shareManager) {
+		IUserSession $userSession,
+		IShareManager $shareManager,
+		IConfig $config
+	) {
 		$this->groupManager = $IGroupManager;
 		$this->userSession = $userSession;
 		$this->shareManager = $shareManager;
+		$this->config = $config;
 	}
 
 	/**
@@ -205,10 +213,14 @@ class GroupPrincipalBackend implements BackendInterface {
 			$restrictGroups = $this->groupManager->getUserGroupIds($user);
 		}
 
+		$searchLimit = $this->config->getSystemValueInt('sharing.maxAutocompleteResults', Constants::SHARING_MAX_AUTOCOMPLETE_RESULTS_DEFAULT);
+		if ($searchLimit <= 0) {
+			$searchLimit = null;
+		}
 		foreach ($searchProperties as $prop => $value) {
 			switch ($prop) {
 				case '{DAV:}displayname':
-					$groups = $this->groupManager->search($value);
+					$groups = $this->groupManager->search($value, $searchLimit);
 
 					$results[] = array_reduce($groups, function (array $carry, IGroup $group) use ($restrictGroups) {
 						$gid = $group->getGID();

apps/dav/lib/RootCollection.php

@@ -72,7 +72,7 @@ class RootCollection extends SimpleCollection {
 			$proxyMapper,
 			\OC::$server->getConfig()
 		);
-		$groupPrincipalBackend = new GroupPrincipalBackend($groupManager, $userSession, $shareManager);
+		$groupPrincipalBackend = new GroupPrincipalBackend($groupManager, $userSession, $shareManager, $config);
 		$calendarResourcePrincipalBackend = new ResourcePrincipalBackend($db, $userSession, $groupManager, $logger, $proxyMapper);
 		$calendarRoomPrincipalBackend = new RoomPrincipalBackend($db, $userSession, $groupManager, $logger, $proxyMapper);
 		// as soon as debug mode is enabled we allow listing of principals

apps/dav/tests/unit/DAV/GroupPrincipalTest.php

@@ -31,6 +31,7 @@ namespace OCA\DAV\Tests\unit\DAV;
 
 use OC\Group\Group;
 use OCA\DAV\DAV\GroupPrincipalBackend;
+use OCP\IConfig;
 use OCP\IGroup;
 use OCP\IGroupManager;
 use OCP\IUser;
@@ -39,6 +40,8 @@ use OCP\Share\IManager;
 use Sabre\DAV\PropPatch;
 
 class GroupPrincipalTest extends \Test\TestCase {
+	/** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */
+	private $config;
 
 	/** @var IGroupManager | \PHPUnit\Framework\MockObject\MockObject */
 	private $groupManager;
@@ -56,11 +59,14 @@ class GroupPrincipalTest extends \Test\TestCase {
 		$this->groupManager = $this->createMock(IGroupManager::class);
 		$this->userSession = $this->createMock(IUserSession::class);
 		$this->shareManager = $this->createMock(IManager::class);
+		$this->config = $this->createMock(IConfig::class);
 
 		$this->connector = new GroupPrincipalBackend(
 			$this->groupManager,
 			$this->userSession,
-			$this->shareManager);
+			$this->shareManager,
+			$this->config
+		);
 		parent::setUp();
 	}
 
@@ -167,6 +173,23 @@ class GroupPrincipalTest extends \Test\TestCase {
 		$this->assertSame($expectedResponse, $response);
 	}
 
+	public function testGetPrincipalsByPathGroupWithHash() {
+		$group1 = $this->mockGroup('foo#bar');
+		$this->groupManager
+			->expects($this->once())
+			->method('get')
+			->with('foo#bar')
+			->willReturn($group1);
+
+		$expectedResponse = [
+			'uri' => 'principals/groups/foo%23bar',
+			'{DAV:}displayname' => 'Group foo#bar',
+			'{urn:ietf:params:xml:ns:caldav}calendar-user-type' => 'GROUP',
+		];
+		$response = $this->connector->getPrincipalByPath('principals/groups/foo#bar');
+		$this->assertSame($expectedResponse, $response);
+	}
+
 	public function testGetGroupMemberSet() {
 		$response = $this->connector->getGroupMemberSet('principals/groups/foo');
 		$this->assertSame([], $response);

apps/files_sharing/lib/Controller/ShareesAPIController.php

@@ -36,6 +36,7 @@ declare(strict_types=1);
 
 namespace OCA\Files_Sharing\Controller;
 
+use OCP\Constants;
 use function array_slice;
 use function array_values;
 use Generator;
@@ -148,7 +149,7 @@ class ShareesAPIController extends OCSController {
 		}
 
 		// never return more than the max. number of results configured in the config.php
-		$maxResults = (int)$this->config->getSystemValue('sharing.maxAutocompleteResults', 0);
+		$maxResults = $this->config->getSystemValueInt('sharing.maxAutocompleteResults', Constants::SHARING_MAX_AUTOCOMPLETE_RESULTS_DEFAULT);
 		if ($maxResults > 0) {
 			$perPage = min($perPage, $maxResults);
 		}

apps/files_sharing/src/services/ConfigService.js

@@ -248,7 +248,7 @@ export default class Config {
 	 * @memberof Config
 	 */
 	get maxAutocompleteResults() {
-		return parseInt(OC.config['sharing.maxAutocompleteResults'], 10) || 200
+		return parseInt(OC.config['sharing.maxAutocompleteResults'], 10) || 25
 	}
 
 	/**

config/config.sample.php

@@ -1410,10 +1410,17 @@ $CONFIG = [
 'sharing.managerFactory' => '\OC\Share20\ProviderFactory',
 
 /**
- * Define max number of results returned by the user search for auto-completion
+ * Define max number of results returned by the search for auto-completion of
- * Default is unlimited (value set to 0).
+ * users, groups, etc. The value must not be lower than 0 (for unlimited).
+ *
+ * If more, different sources are requested (e.g. different user backends; or
+ * both users and groups), the value is applied per source and might not be
+ * truncated after collecting the results. I.e. more results can appear than
+ * configured here.
+ *
+ * Default is 25.
  */
-'sharing.maxAutocompleteResults' => 0,
+'sharing.maxAutocompleteResults' => 25,
 
 /**
  * Define the minimum length of the search string before we start auto-completion

lib/private/Contacts/ContactsMenu/Manager.php

@@ -27,6 +27,7 @@
 namespace OC\Contacts\ContactsMenu;
 
 use OCP\App\IAppManager;
+use OCP\Constants;
 use OCP\Contacts\ContactsMenu\IEntry;
 use OCP\IConfig;
 use OCP\IUser;
@@ -63,7 +64,7 @@ class Manager {
 	 * @return array
 	 */
 	public function getEntries(IUser $user, $filter) {
-		$maxAutocompleteResults = $this->config->getSystemValueInt('sharing.maxAutocompleteResults', 25);
+		$maxAutocompleteResults = max(0, $this->config->getSystemValueInt('sharing.maxAutocompleteResults', Constants::SHARING_MAX_AUTOCOMPLETE_RESULTS_DEFAULT));
 		$minSearchStringLength = $this->config->getSystemValueInt('sharing.minSearchStringLength', 0);
 		$topEntries = [];
 		if (strlen($filter) >= $minSearchStringLength) {

lib/private/Template/JSConfigHelper.php

@@ -33,6 +33,7 @@ namespace OC\Template;
 use bantu\IniGetWrapper\IniGetWrapper;
 use OC\CapabilitiesManager;
 use OCP\App\IAppManager;
+use OCP\Constants;
 use OCP\Defaults;
 use OCP\IConfig;
 use OCP\IGroupManager;
@@ -189,8 +190,8 @@ class JSConfigHelper {
 			'enable_avatars' => true, // here for legacy reasons - to not crash existing code that relies on this value
 			'lost_password_link' => $this->config->getSystemValue('lost_password_link', null),
 			'modRewriteWorking' => $this->config->getSystemValue('htaccess.IgnoreFrontController', false) === true || getenv('front_controller_active') === 'true',
-			'sharing.maxAutocompleteResults' => (int)$this->config->getSystemValue('sharing.maxAutocompleteResults', 0),
+			'sharing.maxAutocompleteResults' => max(0, $this->config->getSystemValueInt('sharing.maxAutocompleteResults', Constants::SHARING_MAX_AUTOCOMPLETE_RESULTS_DEFAULT)),
-			'sharing.minSearchStringLength' => (int)$this->config->getSystemValue('sharing.minSearchStringLength', 0),
+			'sharing.minSearchStringLength' => $this->config->getSystemValueInt('sharing.minSearchStringLength', 0),
 			'blacklist_files_regex' => \OCP\Files\FileInfo::BLACKLIST_FILES_REGEX,
 		];
 

lib/public/Constants.php

@@ -52,4 +52,10 @@ class Constants {
 	 * longer support windows as server platform.
 	 */
 	public const FILENAME_INVALID_CHARS = "\\/";
+
+	/**
+	 * @since 21.0.0 – default value for autocomplete/search results limit,
+	 * cf. sharing.maxAutocompleteResults in config.sample.php.
+	 */
+	public const SHARING_MAX_AUTOCOMPLETE_RESULTS_DEFAULT = 25;
 }

tests/lib/Contacts/ContactsMenu/ManagerTest.php

@@ -28,6 +28,7 @@ use OC\Contacts\ContactsMenu\ActionProviderStore;
 use OC\Contacts\ContactsMenu\ContactsStore;
 use OC\Contacts\ContactsMenu\Manager;
 use OCP\App\IAppManager;
+use OCP\Constants;
 use OCP\Contacts\ContactsMenu\IEntry;
 use OCP\Contacts\ContactsMenu\IProvider;
 use OCP\IConfig;
@@ -82,7 +83,7 @@ class ManagerTest extends TestCase {
 
 		$this->config->expects($this->at(0))
 			->method('getSystemValueInt')
-			->with('sharing.maxAutocompleteResults', 25)
+			->with('sharing.maxAutocompleteResults', Constants::SHARING_MAX_AUTOCOMPLETE_RESULTS_DEFAULT)
 			->willReturn(25);
 		$this->config->expects($this->at(1))
 			->method('getSystemValueInt')
@@ -120,7 +121,7 @@ class ManagerTest extends TestCase {
 
 		$this->config->expects($this->at(0))
 			->method('getSystemValueInt')
-			->with('sharing.maxAutocompleteResults', 25)
+			->with('sharing.maxAutocompleteResults', Constants::SHARING_MAX_AUTOCOMPLETE_RESULTS_DEFAULT)
 			->willReturn(3);
 		$this->config->expects($this->at(1))
 			->method('getSystemValueInt')
@@ -157,7 +158,7 @@ class ManagerTest extends TestCase {
 
 		$this->config->expects($this->at(0))
 			->method('getSystemValueInt')
-			->with('sharing.maxAutocompleteResults', 25)
+			->with('sharing.maxAutocompleteResults', Constants::SHARING_MAX_AUTOCOMPLETE_RESULTS_DEFAULT)
 			->willReturn(3);
 		$this->config->expects($this->at(1))
 			->method('getSystemValueInt')