Merge pull request #2560 from nextcloud/acl-preload-rules

pass around preload rules in acl wrapper instead of relying on the cache

lib/ACL/ACLCacheWrapper.php

@@ -30,11 +30,15 @@
 use OCP\Files\Search\ISearchQuery;
 
 class ACLCacheWrapper extends CacheWrapper {
-	private $aclManager;
-	private $inShare;
+	private ACLManager $aclManager;
+	private bool $inShare;
 
-	private function getACLPermissionsForPath(string $path) {
-		$permissions = $this->aclManager->getACLPermissionsForPath($path);
+	private function getACLPermissionsForPath(string $path, array $rules = []) {
+		if ($rules) {
+			$permissions = $this->aclManager->getPermissionsForPathFromRules($path, $rules);
+		} else {
+			$permissions = $this->aclManager->getACLPermissionsForPath($path);
+		}
 
 		// if there is no read permissions, than deny everything
 		if ($this->inShare) {
@@ -52,10 +56,10 @@ public function __construct(ICache $cache, ACLManager $aclManager, bool $inShare
 		$this->inShare = $inShare;
 	}
 
-	protected function formatCacheEntry($entry) {
+	protected function formatCacheEntry($entry, array $rules = []) {
 		if (isset($entry['permissions'])) {
 			$entry['scan_permissions'] = $entry['permissions'];
-			$entry['permissions'] &= $this->getACLPermissionsForPath($entry['path']);
+			$entry['permissions'] &= $this->getACLPermissionsForPath($entry['path'], $rules);
 			if (!$entry['permissions']) {
 				return false;
 			}
@@ -65,8 +69,10 @@ protected function formatCacheEntry($entry) {
 
 	public function getFolderContentsById($fileId) {
 		$results = $this->getCache()->getFolderContentsById($fileId);
-		$this->preloadEntries($results);
-		$entries = array_map([$this, 'formatCacheEntry'], $results);
+		$rules = $this->preloadEntries($results);
+		$entries = array_map(function ($entry) use ($rules) {
+			return $this->formatCacheEntry($entry, $rules);
+		}, $results);
 		return array_filter(array_filter($entries));
 	}
 
@@ -90,11 +96,12 @@ public function searchQuery(ISearchQuery $query) {
 
 	/**
 	 * @param ICacheEntry[] $entries
+	 * @return Rule[][]
 	 */
-	private function preloadEntries(array $entries) {
+	private function preloadEntries(array $entries): array {
 		$paths = array_map(function (ICacheEntry $entry) {
 			return $entry->getPath();
 		}, $entries);
-		$this->aclManager->preloadPaths($paths);
+		return $this->aclManager->getRelevantRulesForPath($paths, false);
 	}
 }

lib/ACL/ACLManager.php

@@ -56,10 +56,13 @@ private function getRootStorageId(): int {
 	}
 
 	/**
-	 * @param array $paths
-	 * @return (Rule[])[]
+	 * Get the list of rules applicable for a set of paths
+	 *
+	 * @param string[] $paths
+	 * @param bool $cache whether to cache the retrieved rules
+	 * @return array<string, Rule[]> sorted parent first
 	 */
-	private function getRules(array $paths): array {
+	private function getRules(array $paths, bool $cache = true): array {
 		// beware: adding new rules to the cache besides the cap
 		// might discard former cached entries, so we can't assume they'll stay
 		// cached, so we read everything out initially to be able to return it
@@ -74,7 +77,9 @@ private function getRules(array $paths): array {
 		if (!empty($nonCachedPaths)) {
 			$newRules = $this->ruleManager->getRulesForFilesByPath($this->user, $this->getRootStorageId(), $nonCachedPaths);
 			foreach ($newRules as $path => $rulesForPath) {
-				$this->ruleCache->set($path, $rulesForPath);
+				if ($cache) {
+					$this->ruleCache->set($path, $rulesForPath);
+				}
 				$rules[$path] = $rulesForPath;
 			}
 		}
@@ -85,10 +90,14 @@ private function getRules(array $paths): array {
 	}
 
 	/**
+	 * Get a list of all path that might contain relevant rules when calculating the permissions for a path
+	 *
+	 * This contains the $path itself and any parent folder
+	 *
 	 * @param string $path
 	 * @return string[]
 	 */
-	private function getParents(string $path): array {
+	private function getRelevantPaths(string $path): array {
 		$paths = [$path];
 		while ($path !== '') {
 			$path = dirname($path);
@@ -101,18 +110,47 @@ private function getParents(string $path): array {
 		return $paths;
 	}
 
-	public function preloadPaths(array $paths): void {
+	/**
+	 * Get the list of rules applicable for a set of paths, including rules for any parent
+	 *
+	 * @param string[] $paths
+	 * @param bool $cache whether to cache the retrieved rules
+	 * @return array<string, Rule[]> sorted parent first
+	 */
+	public function getRelevantRulesForPath(array $paths, bool $cache = true): array {
 		$allPaths = [];
 		foreach ($paths as $path) {
-			$allPaths = array_unique(array_merge($allPaths, $this->getParents($path)));
+			$allPaths = array_unique(array_merge($allPaths, $this->getRelevantPaths($path)));
 		}
-		$this->getRules($allPaths);
+		return $this->getRules($allPaths, $cache);
 	}
 
 	public function getACLPermissionsForPath(string $path): int {
 		$path = ltrim($path, '/');
-		$rules = $this->getRules($this->getParents($path));
+		$rules = $this->getRules($this->getRelevantPaths($path));
+
+		return $this->calculatePermissionsForPath($rules);
+	}
+
+	/**
+	 * @param string $path
+	 * @param array<string, Rule[]> $rules list of rules per path
+	 * @return int
+	 */
+	public function getPermissionsForPathFromRules(string $path, array $rules): int {
+		$path = ltrim($path, '/');
+		$relevantPaths = $this->getRelevantPaths($path);
+		$rules = array_intersect_key($rules, array_flip($relevantPaths));
+		return $this->calculatePermissionsForPath($rules);
+	}
 
+	/**
+	 * @param array<string, Rule[]> $rules list of rules per path, sorted parent first
+	 * @return int
+	 */
+	private function calculatePermissionsForPath(array $rules): int {
+		// first combine all rules with the same path, then apply them on top of the current permissions
+		// since $rules is sorted parent first rules for subfolders overwrite the rules from the parent
 		return array_reduce($rules, function (int $permissions, array $rules): int {
 			$mergedRule = Rule::mergeRules($rules);
 			return $mergedRule->applyPermissions($permissions);

lib/ACL/Rule.php

@@ -70,6 +70,14 @@ public function getPermissions(): int {
 		return $this->permissions;
 	}
 
+	/**
+	 * Apply this rule to an existing permission set, returning the resulting permissions
+	 *
+	 * All permissions included in the current mask will overwrite the existing permissions
+	 *
+	 * @param int $permissions
+	 * @return int
+	 */
 	public function applyPermissions(int $permissions): int {
 		$invertedMask = ~$this->mask;
 		// create a bitmask that has all inherit and allow bits set to 1 and all deny bits to 0

lib/Listeners/CircleDestroyedEventListener.php

@@ -32,7 +32,6 @@
 use OCP\EventDispatcher\IEventListener;
 
 class CircleDestroyedEventListener implements IEventListener {
-
 	public function __construct(
 		private FolderManager $folderManager,
 	) {

lib/Migration/Version1401000Date20230426112001.php

@@ -33,7 +33,6 @@
 use OCP\Migration\SimpleMigrationStep;
 
 class Version1401000Date20230426112001 extends SimpleMigrationStep {
-
 	public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
 		/** @var ISchemaWrapper $schema */
 		$schema = $schemaClosure();

lib/Migration/Version16000Date20230821085801.php

@@ -36,7 +36,6 @@
  * Auto-generated migration step: Please modify to your needs!
  */
 class Version16000Date20230821085801 extends SimpleMigrationStep {
-
 	/**
 	 * @param IOutput $output
 	 * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`

lib/Mount/MountProvider.php

@@ -138,9 +138,9 @@ public function getMountsForUser(IUser $user, IStorageFactory $loader) {
 			return $this->getJailPath($folder['folder_id']);
 		}, $foldersWithAcl);
 		$aclManager = $this->aclManagerFactory->getACLManager($user, $this->getRootStorageId());
-		$aclManager->preloadPaths($aclRootPaths);
+		$rootRules = $aclManager->getRelevantRulesForPath($aclRootPaths);
 
-		return array_values(array_filter(array_map(function ($folder) use ($user, $loader, $conflicts, $aclManager) {
+		return array_values(array_filter(array_map(function ($folder) use ($user, $loader, $conflicts, $aclManager, $rootRules) {
 			// check for existing files in the user home and rename them if needed
 			$originalFolderName = $folder['mount_point'];
 			if (in_array($originalFolderName, $conflicts)) {
@@ -168,7 +168,8 @@ public function getMountsForUser(IUser $user, IStorageFactory $loader) {
 				$loader,
 				$folder['acl'],
 				$user,
-				$aclManager
+				$aclManager,
+				$rootRules
 			);
 		}, $folders)));
 	}
@@ -200,7 +201,8 @@ public function getMount(
 		IStorageFactory $loader = null,
 		bool $acl = false,
 		IUser $user = null,
-		?ACLManager $aclManager = null
+		?ACLManager $aclManager = null,
+		array $rootRules = []
 	): ?IMountPoint {
 		if (!$cacheEntry) {
 			// trigger folder creation
@@ -219,12 +221,12 @@ public function getMount(
 		if ($acl && $user) {
 			$inShare = $this->getCurrentUID() === null || $this->getCurrentUID() !== $user->getUID();
 			$aclManager ??= $this->aclManagerFactory->getACLManager($user, $this->getRootStorageId());
+			$aclRootPermissions = $aclManager->getPermissionsForPathFromRules($rootPath, $rootRules);
 			$storage = new ACLStorageWrapper([
 				'storage' => $storage,
 				'acl_manager' => $aclManager,
-				'in_share' => $inShare
+				'in_share' => $inShare,
 			]);
-			$aclRootPermissions = $aclManager->getACLPermissionsForPath($rootPath);
 			$cacheEntry['permissions'] &= $aclRootPermissions;
 		}
 

lib/Settings/Admin.php

@@ -30,7 +30,6 @@
 use OCP\Settings\IDelegatedSettings;
 
 class Admin implements IDelegatedSettings {
-
 	public function __construct(
 		private IInitialState $initialState,
 		private ApplicationService $applicationService,

lib/Versions/GroupVersion.php

@@ -30,7 +30,6 @@
 use OCP\IUser;
 
 class GroupVersion extends Version {
-
 	public function __construct(
 		int $timestamp,
 		int $revisionId,