Correctly apply groupfolders acl

lib/FilesHooks.php

@@ -17,7 +17,11 @@
 use OCP\Activity\IManager;
 use OCP\BackgroundJob\IJobList;
 use OCP\Constants;
+use OCP\Files\Cache\IFileAccess;
+use OCP\Files\Config\ICachedMountFileInfo;
+use OCP\Files\Config\IMountProviderCollection;
 use OCP\Files\Config\IUserMountCache;
+use OCP\Files\Config\MountProviderArgs;
 use OCP\Files\File;
 use OCP\Files\IRootFolder;
 use OCP\Files\Node;
@@ -66,6 +70,8 @@ public function __construct(
 		protected NotificationGenerator $notificationGenerator,
 		protected ITagManager $tagManager,
 		protected ?CirclesManager $teamManager,
+		private IFileAccess $fileAccess,
+		private IMountProviderCollection $mountProviderCollection,
 	) {
 	}
 
@@ -1273,29 +1279,21 @@ private function getAffectedUsersFromCachedMounts(int $fileId): array {
 		$mountsForFile = $this->userMountCache->getMountsForFileId($fileId);
 		foreach ($mountsForFile as $mount) {
 			$affectedUsers[$mount->getUser()->getUID()] = $this->getVisiblePath($mount->getPath());
-			$cachedMounts[] = [
-				'userId' => $mount->getUser()->getUID(),
-				'provider' => str_replace('\\\\', '\\', $mount->getMountProvider()),
-				'path' => $mount->getPath(),
-				'visiblePath' => $this->getVisiblePath($mount->getPath()),
-				'storageId' => $mount->getStorageId(),
-				'internalPath' => $mount->getInternalPath(),
-				'rootInternalPath' => $mount->getRootInternalPath(),
-			];
 		}
 
-		$unrelatedUsers = $this->getUnrelatedUsers($cachedMounts);
+		$unrelatedUsers = $this->getBlockedGroupfoldersAclUsers($mountsForFile);
 
 		return array_filter($affectedUsers, fn (string $userId): bool => !in_array($userId, $unrelatedUsers, true), ARRAY_FILTER_USE_KEY);
 	}
 
 	/**
 	 * returns an array of users that have confirmed no access to fileId
 	 *
+	 * @param ICachedMountFileInfo[] $cachedMounts
 	 *
-	 * @return string[] list of unrelated userIds
+	 * @return string[] list of blocked userIds
 	 */
-	private function getUnrelatedUsers(array $cachedMounts): array {
+	private function getBlockedGroupfoldersAclUsers(array $cachedMounts): array {
 		/** @var \OCA\GroupFolders\ACL\RuleManager $ruleManager */
 		/** @var \OCA\GroupFolders\Folder\FolderManager $folderManager */
 		try {
@@ -1305,54 +1303,66 @@ private function getUnrelatedUsers(array $cachedMounts): array {
 			return []; // if we have no access to RuleManager, we cannot filter unrelated users
 		}
 
-		$groupFolderAclStatus = [];
-
 		/** @var \OCA\GroupFolders\ACL\Rule[] $rules */
 		$rules = $knownRules = $knownGroupRules = $usersToCheck = $cachedPath = [];
+		$groupfolderId = null;
 		foreach ($cachedMounts as $cachedMount) {
 			// we are only interested in filtering GroupFolders ACL
-			if ($cachedMount['provider'] !== 'OCA\GroupFolders\Mount\MountProvider') {
+			if ($cachedMount->getMountProvider() !== 'OCA\GroupFolders\Mount\MountProvider') {
 				continue;
 			}
 
 			// caching rules based on storage id
-			$storageId = $cachedMount['storageId'];
+			$storageId = $cachedMount->getStorageId();
 			if (!array_key_exists($storageId, $knownRules)) {
 				$knownRules[$storageId] = [];
 			}
 
-			$cachedPath[$cachedMount['userId']] = $fullPath = $cachedMount['path'];
+			$cachedPath[$cachedMount->getUser()->getUID()] = $fullPath = $cachedMount->getPath();
 
 			// caching rules based on storage+path to file
-			if (!array_key_exists($cachedMount['visiblePath'], $knownRules[$storageId])) {
+			$visiblePath = $this->getVisiblePath($cachedMount->getPath());
+			if (!array_key_exists($visiblePath, $knownRules[$storageId])) {
 				// we need mountPoint and folderId to generate the correct path
 				try {
-					// only check for groupfolders
-					if (!str_starts_with($cachedMount['rootInternalPath'], '__groupfolders')) {
-						continue;
-					}
-
-					$folderId = (int)basename($cachedMount['rootInternalPath']);
-					if (!isset($groupFolderAclStatus[$folderId])) {
-						$groupFolderAclStatus[$folderId] = $folderManager->getFolderAclEnabled($folderId);
-					}
-					if (!$groupFolderAclStatus[$folderId]) {
-						continue; // acl are disable
+					if ($groupfolderId === null) {
+						$rootId = $cachedMount->getRootId();
+						$rootMetadata = $this->fileAccess->getByFileId($rootId);
+						if (!$rootMetadata) {
+							continue;
+						}
+						$providerArgs = new MountProviderArgs($cachedMount, $rootMetadata);
+						/** @psalm-suppress UndefinedInterfaceMethod Could not find a public API for this */
+						$mounts = $this->mountProviderCollection->getUserMountsFromProviderByPath(
+							$cachedMount->getMountProvider(),
+							'',
+							false,
+							[$providerArgs]
+						);
+						$mount = reset($mounts);
+						if (!($mount instanceof \OCA\GroupFolders\Mount\GroupMountPoint)) {
+							continue;
+						}
+						$groupfolderId = $mount->getFolderId();
+						if (!$folderManager->getFolderAclEnabled($groupfolderId)) {
+							// ACL are disabled for the groupfolder, we can stop here
+							break;
+						}
 					}
 
-					$folderPath = '/' . $cachedMount['rootInternalPath'];
-					$path = $cachedMount['internalPath'];
+					$folderPath = '/' . $cachedMount->getRootInternalPath();
+					$path = $cachedMount->getInternalPath();
 				} catch (\Exception) {
 					// in case of issue during the process, we can imagine the user have no access to the file
-					$usersToCheck[] = $cachedMount['userId'];
+					$usersToCheck[] = $cachedMount->getUser()->getUID();
 					continue; // we'll catch rules on next user with access to the file
 				}
 
 				// we generate a list of path from top level of group folder to the file itself to get all rules
 				$paths = [$folderPath];
 				while ($path !== '') {
 					$paths[] = $folderPath . '/' . $path;
-					$path = dirname((string)$path);
+					$path = dirname($path);
 					if ($path === '.' || $path === '/') {
 						$path = '';
 					}
@@ -1373,7 +1383,7 @@ private function getUnrelatedUsers(array $cachedMounts): array {
 					$rules = array_merge($rules, $rulesPerPath[$path]);
 				}
 
-				$knownRules[$storageId][$cachedMount['visiblePath']] = true;
+				$knownRules[$storageId][$visiblePath] = true;
 			}
 		}
 
@@ -1412,7 +1422,7 @@ private function getUnrelatedUsers(array $cachedMounts): array {
 		// now that we have a list of eventuals filtered users, we confirm they have no access to the file
 		$filteredUsers = [];
 		foreach ($usersToCheck as $userId) {
-			if (!array_key_exists($userId, $cachedPath) || $cachedPath[$userId] === null) {
+			if (!array_key_exists($userId, $cachedPath)) {
 				$this->logger->notice('could not find user in list of cached path', ['cachePath' => $cachedPath, 'usersToCheck' => $usersToCheck, 'current' => $userId]);
 				$filteredUsers[] = $userId;
 				continue;

psalm.xml

@@ -25,13 +25,29 @@
 		<MissingImmutableAnnotation errorLevel="suppress"/>
 	</issueHandlers>
 	<stubs>
-		<file name="tests/stubs/oc_core_command_base.php" preloadClasses="true" />
-		<file name="tests/stubs/oc_files.php" preloadClasses="true" />
-		<file name="tests/stubs/oca_files_event.php" preloadClasses="true" />
-		<file name="tests/stubs/oca_files_sharing.php" preloadClasses="true" />
-		<file name="tests/stubs/oca_files_trashbin_events.php" preloadClasses="true" />
-		<file name="tests/stubs/oca_groupfolders.php" preloadClasses="true" />
-		<file name="tests/stubs/oca_viewer_event.php" preloadClasses="true" />
-		<file name="tests/stubs/oca_theming_defaults.php" preloadClasses="true" />
+		<file name="tests/stubs/oc_core_command_base.php" preloadClasses="true"/>
+		<file name="tests/stubs/oc_defaults.php" preloadClasses="true"/>
+		<file name="tests/stubs/oc_files_mount_mountpoint.php" preloadClasses="true"/>
+		<file name="tests/stubs/oc_files_storage_common.php" preloadClasses="true"/>
+		<file name="tests/stubs/oc_files_storage_dav.php" preloadClasses="true"/>
+		<file name="tests/stubs/oc_files_storage_storage.php" preloadClasses="true"/>
+		<file name="tests/stubs/oc_files_storage_wrapper_wrapper.php" preloadClasses="true"/>
+		<file name="tests/stubs/oca_files_event_loadsidebar.php" preloadClasses="true"/>
+		<file name="tests/stubs/oca_files_sharing_external_storage.php" preloadClasses="true"/>
+		<file name="tests/stubs/oca_files_sharing_isharedmountpoint.php" preloadClasses="true"/>
+		<file name="tests/stubs/oca_files_sharing_isharedstorage.php" preloadClasses="true"/>
+		<file name="tests/stubs/oca_files_sharing_sharedmount.php" preloadClasses="true"/>
+		<file name="tests/stubs/oca_files_trashbin_events_noderestoredevent.php" preloadClasses="true"/>
+		<file name="tests/stubs/oca_groupfolders_acl_rule.php" preloadClasses="true"/>
+		<file name="tests/stubs/oca_groupfolders_acl_rulemanager.php" preloadClasses="true"/>
+		<file name="tests/stubs/oca_groupfolders_acl_usermapping_iusermapping.php" preloadClasses="true"/>
+		<file name="tests/stubs/oca_groupfolders_folder_foldermanager.php" preloadClasses="true"/>
+		<file name="tests/stubs/oca_groupfolders_mount_groupmountpoint.php" preloadClasses="true"/>
+		<file name="tests/stubs/oca_theming_themingdefaults.php" preloadClasses="true"/>
+		<file name="tests/stubs/oca_viewer_event_loadviewer.php" preloadClasses="true"/>
+		<file name="tests/stubs/sabre_xml_xmldeserializable.php" preloadClasses="true"/>
+		<file name="tests/stubs/sabre_xml_xmlserializable.php" preloadClasses="true"/>
+		<file name="tests/stubs/stecman_component_symfony_console_bashcompletion_completion_completionawareinterface.php" preloadClasses="true"/>
+		<file name="tests/stubs/stecman_component_symfony_console_bashcompletion_completioncontext.php" preloadClasses="true"/>
 	</stubs>
 </psalm>

tests/FilesHooksTest.php

@@ -33,6 +33,8 @@
 use OCA\Files_Sharing\SharedMount;
 use OCP\Activity\IEvent;
 use OCP\Activity\IManager;
+use OCP\Files\Cache\IFileAccess;
+use OCP\Files\Config\IMountProviderCollection;
 use OCP\Files\Config\IUserMountCache;
 use OCP\Files\File;
 use OCP\Files\Folder;
@@ -77,6 +79,8 @@ class FilesHooksTest extends TestCase {
 	protected TagManager&MockObject $tagManager;
 	protected Tags&MockObject $tags;
 	protected (OCA\Circles\CirclesManager&MockObject)|null $teamManager = null;
+	private IFileAccess&MockObject $fileAccess;
+	private IMountProviderCollection&MockObject $mountProviderCollection;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -98,6 +102,8 @@ protected function setUp(): void {
 			->willReturn([]);
 		$this->tagManager->method('load')
 			->willReturn($this->tags);
+		$this->fileAccess = $this->createMock(IFileAccess::class);
+		$this->mountProviderCollection = $this->createMock(IMountProviderCollection::class);
 
 		$this->filesHooks = $this->getFilesHooks();
 	}
@@ -131,6 +137,8 @@ protected function getFilesHooks(array $mockedMethods = [], ?string $user = 'use
 					$this->notificationGenerator,
 					$this->tagManager,
 					$this->teamManager,
+					$this->fileAccess,
+					$this->mountProviderCollection,
 				])
 				->onlyMethods($mockedMethods)
 				->getMock();
@@ -152,6 +160,8 @@ protected function getFilesHooks(array $mockedMethods = [], ?string $user = 'use
 			$this->notificationGenerator,
 			$this->tagManager,
 			$this->teamManager,
+			$this->fileAccess,
+			$this->mountProviderCollection,
 		);
 	}
 
@@ -199,6 +209,8 @@ public function testFileCreate(string $currentUser, bool $isPublicShare, string
 				$this->notificationGenerator,
 				$this->tagManager,
 				$this->teamManager,
+				$this->fileAccess,
+				$this->mountProviderCollection,
 			])
 			->onlyMethods(['addNotificationsForFileAction'])
 			->getMock();

tests/stubs/oc_core_command_base.php

@@ -1,49 +1,89 @@
 <?php
 
-namespace Stecman\Component\Symfony\Console\BashCompletion {
-	class CompletionContext {
+/**
+ * SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+namespace OC\Core\Command;
+
+use Stecman\Component\Symfony\Console\BashCompletion\Completion\CompletionAwareInterface;
+use Stecman\Component\Symfony\Console\BashCompletion\CompletionContext;
+use Symfony\Component\Console\Command\Command;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Output\OutputInterface;
+
+class Base extends Command implements CompletionAwareInterface {
+	public const OUTPUT_FORMAT_PLAIN = 'plain';
+	public const OUTPUT_FORMAT_JSON = 'json';
+	public const OUTPUT_FORMAT_JSON_PRETTY = 'json_pretty';
+
+	protected string $defaultOutputFormat = self::OUTPUT_FORMAT_PLAIN;
+
+	#[\Override]
+	protected function configure() {
 	}
-}
 
-namespace Stecman\Component\Symfony\Console\BashCompletion\Completion {
-	interface CompletionAwareInterface {
-		/**
-		 * @param string $optionName
-		 * @return array
-		 */
-		public function completeOptionValues($optionName, \Stecman\Component\Symfony\Console\BashCompletion\CompletionContext $context);
+	protected function writeArrayInOutputFormat(InputInterface $input, OutputInterface $output, iterable $items, string $prefix = '  - '): void {
+	}
 
-		/**
-		 * @param string $argumentName
-		 * @return array
-		 */
-		public function completeArgumentValues($argumentName, \Stecman\Component\Symfony\Console\BashCompletion\CompletionContext $context);
+	protected function writeTableInOutputFormat(InputInterface $input, OutputInterface $output, array $items): void {
+	}
+
+	protected function writeStreamingTableInOutputFormat(InputInterface $input, OutputInterface $output, \Iterator $items, int $tableGroupSize): void {
+	}
+
+	protected function writeStreamingJsonArray(InputInterface $input, OutputInterface $output, \Iterator $items): void {
+	}
+
+	public function chunkIterator(\Iterator $iterator, int $count): \Iterator {
+	}
+
+	/**
+	 * @param mixed $item
+	 */
+	protected function writeMixedInOutputFormat(InputInterface $input, OutputInterface $output, $item) {
+	}
+
+	protected function valueToString($value, bool $returnNull = true): ?string {
+	}
+
+	/**
+	 * Throw InterruptedException when interrupted by user
+	 *
+	 * @throws InterruptedException
+	 */
+	protected function abortIfInterrupted() {
+	}
+
+	/**
+	 * Changes the status of the command to "interrupted" if ctrl-c has been pressed
+	 *
+	 * Gives a chance to the command to properly terminate what it's doing
+	 */
+	public function cancelOperation(): void {
+	}
+
+	#[\Override]
+	public function run(InputInterface $input, OutputInterface $output): int {
+	}
+
+	/**
+	 * @param string $optionName
+	 * @param CompletionContext $context
+	 * @return string[]
+	 */
+	#[\Override]
+	public function completeOptionValues($optionName, CompletionContext $context) {
 	}
-}
 
-namespace OC\Core\Command {
-	use Stecman\Component\Symfony\Console\BashCompletion\Completion\CompletionAwareInterface;
-	use Stecman\Component\Symfony\Console\BashCompletion\CompletionContext;
-
-	class Base extends \Symfony\Component\Console\Command\Command implements CompletionAwareInterface {
-		public const OUTPUT_FORMAT_PLAIN = 'plain';
-		public const OUTPUT_FORMAT_JSON = 'json';
-		public const OUTPUT_FORMAT_JSON_PRETTY = 'json_pretty';
-
-		protected string $defaultOutputFormat = self::OUTPUT_FORMAT_PLAIN;
-
-		/**
-		 * @param string $optionName
-		 * @return string[]
-		 */
-		public function completeOptionValues($optionName, CompletionContext $context) {
-		}
-
-		/**
-		 * @param string $argumentName
-		 * @return string[]
-		 */
-		public function completeArgumentValues($argumentName, CompletionContext $context) {
-		}
+	/**
+	 * @param string $argumentName
+	 * @param CompletionContext $context
+	 * @return string[]
+	 */
+	#[\Override]
+	public function completeArgumentValues($argumentName, CompletionContext $context) {
 	}
 }