[stable34] filter ACL forbidden paths from search query

lib/ACL/ACLCacheWrapper.php

@@ -9,9 +9,14 @@
 namespace OCA\GroupFolders\ACL;
 
 use OC\Files\Cache\Wrapper\CacheWrapper;
+use OC\Files\Search\SearchBinaryOperator;
+use OC\Files\Search\SearchComparison;
 use OCP\Constants;
 use OCP\Files\Cache\ICache;
 use OCP\Files\Cache\ICacheEntry;
+use OCP\Files\Search\ISearchBinaryOperator;
+use OCP\Files\Search\ISearchComparison;
+use OCP\Files\Search\ISearchOperator;
 use OCP\Files\Search\ISearchQuery;
 
 class ACLCacheWrapper extends CacheWrapper {
@@ -75,23 +80,23 @@ public function getFolderContentsById($fileId, ?string $mimeTypeFilter = null):
 
 	#[\Override]
 	public function search($pattern): array {
-		$results = $this->getCache()->search($pattern);
+		$results = parent::search($pattern);
 		$this->preloadEntries($results);
 
 		return array_filter(array_map($this->formatCacheEntry(...), $results));
 	}
 
 	#[\Override]
 	public function searchByMime($mimetype): array {
-		$results = $this->getCache()->searchByMime($mimetype);
+		$results = parent::searchByMime($mimetype);
 		$this->preloadEntries($results);
 
 		return array_filter(array_map($this->formatCacheEntry(...), $results));
 	}
 
 	#[\Override]
 	public function searchQuery(ISearchQuery $query): array {
-		$results = $this->getCache()->searchQuery($query);
+		$results = parent::searchQuery($query);
 		$this->preloadEntries($results);
 
 		return array_filter(array_map($this->formatCacheEntry(...), $results));
@@ -106,4 +111,32 @@ private function preloadEntries(array $entries): array {
 
 		return $this->aclManager->getRelevantRulesForPath($this->getNumericStorageId(), $paths, false);
 	}
+
+	/**
+	 * Construct a search operator that filters out any paths that the current user doesn't have read permissions for
+	 */
+	private function getSearchFilter(): ISearchOperator {
+		$forbiddenPaths = $this->aclManager->getForbiddenPaths($this->getNumericStorageId(), '');
+
+		$filters = array_map(fn (string $path) => new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_NOT, [
+			new SearchComparison(ISearchComparison::COMPARE_LIKE_CASE_SENSITIVE, 'path', SearchComparison::escapeLikeParameter($path) . '/%')
+		]), $forbiddenPaths);
+		$filters[] = new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_NOT, [
+			new SearchComparison(ISearchComparison::COMPARE_IN, 'path', $forbiddenPaths)
+		]);
+		return new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, $filters);
+	}
+
+	#[\Override]
+	public function getQueryFilterForStorage(): ISearchOperator {
+		$storageFilter = parent::getQueryFilterForStorage();
+
+		return new SearchBinaryOperator(
+			ISearchBinaryOperator::OPERATOR_AND,
+			[
+				$storageFilter,
+				$this->getSearchFilter(),
+			]
+		);
+	}
 }

lib/ACL/ACLManager.php

@@ -293,4 +293,23 @@ public function getBasePermission(int $folderId): int {
 
 		return Constants::PERMISSION_ALL;
 	}
+
+	/**
+	 * Calculate all paths that the current user doesn't have access to.
+	 *
+	 * @return list<string>
+	 */
+	public function getForbiddenPaths(int $storageId, string $prefix): array {
+		$rules = $this->ruleManager->getRulesForPrefix($this->user, $storageId, $prefix);
+		$forbidden = [];
+
+		foreach ($rules as $path => $pathRules) {
+			$mergedRule = Rule::mergeRules($pathRules);
+			if ($mergedRule->applyPermissions(Constants::PERMISSION_READ) === 0) {
+				$forbidden[] = $path;
+			}
+		}
+
+		return $forbidden;
+	}
 }

lib/ACL/RuleManager.php

@@ -280,16 +280,19 @@ public function getRulesForPrefix(IUser $user, int $storageId, string $prefix):
 		$query->select(['f.fileid', 'mapping_type', 'mapping_id', 'mask', 'a.permissions', 'f.path'])
 			->from('group_folders_acl', 'a')
 			->innerJoin('a', 'filecache', 'f', $query->expr()->eq('f.fileid', 'a.fileid'))
-			->where($query->expr()->orX(
-				$query->expr()->like('f.path', $query->createNamedParameter($this->connection->escapeLikeParameter($prefix) . '/%')),
-				$query->expr()->eq('f.path_hash', $query->createNamedParameter(md5($prefix)))
-			))
-			->andWhere($query->expr()->eq('f.storage', $query->createNamedParameter($storageId, IQueryBuilder::PARAM_INT)))
+			->where($query->expr()->eq('f.storage', $query->createNamedParameter($storageId, IQueryBuilder::PARAM_INT)))
 			->andWhere($query->expr()->orX(...array_map(fn (IUserMapping $userMapping): ICompositeExpression => $query->expr()->andX(
 				$query->expr()->eq('mapping_type', $query->createNamedParameter($userMapping->getType())),
 				$query->expr()->eq('mapping_id', $query->createNamedParameter($userMapping->getId()))
 			), $userMappings)));
 
+		if ($prefix !== '') {
+			$query = $query->andWhere($query->expr()->orX(
+				$query->expr()->like('f.path', $query->createNamedParameter($this->connection->escapeLikeParameter($prefix) . '/%')),
+				$query->expr()->eq('f.path_hash', $query->createNamedParameter(md5($prefix)))
+			));
+		}
+
 		$rows = $query->executeQuery()->fetchAll();
 
 		/** @var list<array{fileid: int|string, mapping_type: 'circle'|'group'|'user', mapping_id: string, mask: int|string, permissions: int|string, path: string}> $rows */

phpstan.neon

@@ -44,6 +44,10 @@ parameters:
 		- tests/stubs/oc_files_objectstore_objectstorescanner.php
 		- tests/stubs/oc_files_objectstore_objectstorestorage.php
 		- tests/stubs/oc_files_objectstore_primaryobjectstoreconfig.php
+		- tests/stubs/oc_files_search_searchbinaryoperator.php
+		- tests/stubs/oc_files_search_searchcomparison.php
+		- tests/stubs/oc_files_search_searchorder.php
+		- tests/stubs/oc_files_search_searchquery.php
 		- tests/stubs/oc_files_setupmanager.php
 		- tests/stubs/oc_files_storage_common.php
 		- tests/stubs/oc_files_storage_local.php

tests/ACL/ACLCacheWrapperTest.php

@@ -9,30 +9,71 @@
 namespace OCA\groupfolders\tests\ACL;
 
 use OC\Files\Cache\CacheEntry;
+use OC\Files\Search\SearchComparison;
+use OC\Files\Search\SearchOrder;
+use OC\Files\Search\SearchQuery;
+use OC\Files\Storage\Temporary;
 use OCA\GroupFolders\ACL\ACLCacheWrapper;
 use OCA\GroupFolders\ACL\ACLManager;
+use OCA\GroupFolders\ACL\Rule;
+use OCA\GroupFolders\ACL\RuleManager;
+use OCA\GroupFolders\ACL\UserMapping\IUserMapping;
+use OCA\GroupFolders\ACL\UserMapping\IUserMappingManager;
 use OCP\Constants;
 use OCP\Files\Cache\ICache;
+use OCP\Files\Cache\ICacheEntry;
+use OCP\Files\Search\ISearchComparison;
+use OCP\Files\Search\ISearchOrder;
+use OCP\IUser;
 use PHPUnit\Framework\MockObject\MockObject;
 use Test\TestCase;
 
 /**
  * @group DB
  */
 class ACLCacheWrapperTest extends TestCase {
-	private ACLManager&MockObject $aclManager;
+	private ACLManager $aclManager;
 	private ICache&MockObject $source;
 	private ACLCacheWrapper $cache;
 	/** @var array<string, int> */
 	private array $aclPermissions = [];
 
+	private function makeRule(int $permission): Rule {
+		return new Rule($this->createMock(IUserMapping::class), 0, Constants::PERMISSION_ALL, $permission);
+	}
+
+	/**
+	 * @param string[] $paths
+	 * @return array<string, Rule[]>
+	 */
+	private function getRulesForPaths(IUser $user, int $storageId, array $paths): array {
+		return array_combine($paths, array_map(
+			fn (string $path) => isset($this->aclPermissions[$path]) ? [$this->makeRule($this->aclPermissions[$path])] : [],
+			$paths
+		));
+	}
+
 	#[\Override]
 	protected function setUp(): void {
 		parent::setUp();
 
-		$this->aclManager = $this->createMock(ACLManager::class);
-		$this->aclManager->method('getACLPermissionsForPath')
-			->willReturnCallback(fn (int $folderId, int $storageId, string $path) => $this->aclPermissions[$path] ?? Constants::PERMISSION_ALL);
+		$user = $this->createMock(IUser::class);
+		$ruleManager = $this->createMock(RuleManager::class);
+		$ruleManager->method('getRulesForFilesByPath')
+			->willReturnCallback($this->getRulesForPaths(...));
+		$ruleManager->method('getRulesForPrefix')
+			->willReturnCallback(fn (IUser $user, int $storageId, string $prefix) => array_map(
+				fn (int $permission) => [$this->makeRule($permission)],
+				array_filter(
+					$this->aclPermissions,
+					fn (string $path) => str_starts_with($path, $prefix),
+					ARRAY_FILTER_USE_KEY,
+				))
+			);
+		$mappingManager = $this->createMock(IUserMappingManager::class);
+
+		$this->aclManager = new ACLManager($ruleManager, $mappingManager, $user);
+
 		$this->source = $this->createMock(ICache::class);
 		$this->source->method('getNumericStorageId')
 			->willReturn(1);
@@ -76,4 +117,34 @@ public function testHideNonRead(): void {
 
 		$this->assertEquals($expected, $result);
 	}
+
+	public function testSearchNonRead(): void {
+		$sourceStorage = new Temporary([]);
+		$sourceStorage->mkdir('foo');
+		$sourceStorage->touch('foo/test1.txt', 100);
+		$sourceStorage->touch('foo/test2.txt', 101);
+		$sourceStorage->touch('foo/test3.txt', 102);
+		$sourceStorage->mkdir('bar');
+		$sourceStorage->touch('bar/test1.txt', 200);
+		$sourceStorage->touch('bar/test2.txt', 201);
+		$sourceStorage->touch('bar/test3.txt', 202);
+		$sourceStorage->getScanner()->scan('');
+		$this->aclPermissions['bar'] = 0;
+
+		$cache = new ACLCacheWrapper($sourceStorage->getCache(), $this->aclManager, 0, false);
+
+		$query = new SearchQuery(
+			new SearchComparison(ISearchComparison::COMPARE_LIKE, 'name', '%test%'),
+			2,
+			0,
+			[new SearchOrder(ISearchOrder::DIRECTION_DESCENDING, 'mtime')]
+		);
+		$result = $cache->searchQuery($query);
+		$paths = array_map(fn (ICacheEntry $entry) => $entry->getPath(), $result);
+
+		$this->assertEquals([
+			'foo/test3.txt',
+			'foo/test2.txt'
+		], $paths);
+	}
 }

tests/stubs/oc_app.php

@@ -8,20 +8,16 @@
  * SPDX-License-Identifier: AGPL-3.0-only
  */
 use OC\App\AppManager;
-use OC\App\DependencyAnalyzer;
 use OC\AppFramework\App;
 use OC\AppFramework\Bootstrap\Coordinator;
 use OC\Installer;
 use OC\NeedsUpdateException;
-use OC\Repair;
-use OC\Repair\Events\RepairErrorEvent;
 use OC\SystemConfig;
 use OCP\App\AppPathNotFoundException;
 use OCP\App\IAppManager;
 use OCP\Authentication\IAlternativeLogin;
 use OCP\Authentication\IAlternativeLoginProvider;
 use OCP\BackgroundJob\IJobList;
-use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IAppConfig;
 use OCP\IConfig;
 use OCP\IGroup;
@@ -34,7 +30,6 @@
 use OCP\Support\Subscription\IRegistry;
 use Psr\Container\ContainerExceptionInterface;
 use Psr\Log\LoggerInterface;
-use function OCP\Log\logger;
 
 /**
  * This class manages the apps. It allows them to register and integrate in the
@@ -119,6 +114,7 @@ public static function isType(string $app, array $types): bool
 	 * @param bool $all whether to return apps for all users, not only the
 	 *                  currently logged in one
 	 * @return list<string>
+	 * @deprecated 32.0.0 - use {@see \OCP\App\IAppManager::getEnabledAppsForUser} or {@see \OC\OCP\AppIAppManager::getEnabledApps} instead
 	 */
 	public static function getEnabledApps(bool $forceRefresh = false, bool $all = false): array
     {
@@ -256,6 +252,7 @@ public static function updateApp(string $appId): bool
 	 * @param string $appId
 	 * @param string[] $steps
 	 * @throws NeedsUpdateException
+	 * @deprecated 34.0.0 Use {@see \OC\App\AppManager::executeRepairSteps}
 	 */
 	public static function executeRepairSteps(string $appId, array $steps)
     {
@@ -270,6 +267,7 @@ public static function setupBackgroundJobs(array $jobs): void
 
 	/**
 	 * @throws \Exception
+	 * @deprecated 34.0.0 Use {@see \OC\App\AppManager::checkAppDependencies} instead
 	 */
 	public static function checkAppDependencies(IConfig $config, IL10N $l, array $info, bool $ignoreMax): void
     {

tests/stubs/oc_appframework_ocs_baseresponse.php

@@ -4,6 +4,7 @@
  * SPDX-FileCopyrightText: 2016 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
+
 namespace OC\AppFramework\OCS;
 
 use OCP\AppFramework\Http;

tests/stubs/oc_appframework_ocs_v1response.php

@@ -6,6 +6,7 @@
  * SPDX-FileCopyrightText: 2016 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
+
 namespace OC\AppFramework\OCS;
 
 use OCP\AppFramework\Http;

tests/stubs/oc_appframework_utility_simplecontainer.php

@@ -5,6 +5,7 @@
  * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
  * SPDX-License-Identifier: AGPL-3.0-only
  */
+
 namespace OC\AppFramework\Utility;
 
 use ArrayAccess;
@@ -26,6 +27,7 @@
  * SimpleContainer is a simple implementation of a container on basis of Pimple
  */
 class SimpleContainer implements ArrayAccess, ContainerInterface, IContainer {
+	/** @psalm-suppress ImpureStaticProperty A static property is the only way to pass the information from config to autoload */
 	public static bool $useLazyObjects = false;
 
 	public function __construct()