[stable35] Correctly apply groupfolders acl

lib/Command/SendEmails.php

@@ -36,6 +36,7 @@ public function __construct(
 		$this->logger = $logger;
 	}
 
+	#[\Override]
 	protected function configure() {
 		$this
 			->setName('activity:send-mails')
@@ -61,6 +62,7 @@ protected function configure() {
 	 * @param OutputInterface $output
 	 * @return int
 	 */
+	#[\Override]
 	protected function execute(InputInterface $input, OutputInterface $output): int {
 		// We don't use time() but "time() - 1" here, so we don't run into
 		// runtime issues later and delete emails, which were created in the
@@ -98,6 +100,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 	 * @param CompletionContext $context
 	 * @return string[]
 	 */
+	#[\Override]
 	public function completeArgumentValues($argumentName, CompletionContext $context) {
 		if ($argumentName === 'restrict-batching') {
 			return ['asap', 'hourly', 'daily', 'weekly'];

lib/FilesHooks.php

@@ -19,7 +19,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;
@@ -69,6 +73,8 @@ public function __construct(
 		protected NotificationGenerator $notificationGenerator,
 		protected ITagManager $tagManager,
 		protected ?CirclesManager $teamManager,
+		private IFileAccess $fileAccess,
+		private IMountProviderCollection $mountProviderCollection,
 	) {
 	}
 
@@ -1304,18 +1310,9 @@ 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($fileId, $cachedMounts);
+		$unrelatedUsers = $this->getBlockedGroupfoldersAclUsers($mountsForFile);
 
 		return array_filter($affectedUsers, function (string $userId) use ($unrelatedUsers): bool {
 			return !in_array($userId, $unrelatedUsers, true);
@@ -1325,12 +1322,11 @@ private function getAffectedUsersFromCachedMounts(int $fileId): array {
 	/**
 	 * returns an array of users that have confirmed no access to fileId
 	 *
-	 * @param int $fileId
-	 * @param array $cachedMounts
+	 * @param ICachedMountFileInfo[] $cachedMounts
 	 *
-	 * @return string[] list of unrelated userIds
+	 * @return string[] list of blocked userIds
 	 */
-	private function getUnrelatedUsers(int $fileId, array $cachedMounts): array {
+	private function getBlockedGroupfoldersAclUsers(array $cachedMounts): array {
 		/** @var \OCA\GroupFolders\ACL\RuleManager $ruleManager */
 		/** @var \OCA\GroupFolders\Folder\FolderManager $folderManager */
 		try {
@@ -1340,46 +1336,58 @@ private function getUnrelatedUsers(int $fileId, 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'];
-				} catch (\Exception $e) {
+					$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
 				}
 
@@ -1408,7 +1416,7 @@ private function getUnrelatedUsers(int $fileId, array $cachedMounts): array {
 					$rules = array_merge($rules, $rulesPerPath[$path]);
 				}
 
-				$knownRules[$storageId][$cachedMount['visiblePath']] = true;
+				$knownRules[$storageId][$visiblePath] = true;
 			}
 		}
 
@@ -1449,7 +1457,7 @@ private function getUnrelatedUsers(int $fileId, 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

@@ -1,3 +1,3 @@
 <?xml version="1.0"?>
 <psalm
 	errorLevel="4"
@@ -48,10 +48,29 @@
 		</UndefinedDocblockClass>
 	</issueHandlers>
 	<stubs>
-		<file name="tests/stubs/oc_hooks_emitter.php" />
-		<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_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

@@ -34,6 +34,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;
+	private IFileAccess&MockObject $fileAccess;
+	private IMountProviderCollection&MockObject $mountProviderCollection;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -99,6 +103,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();
 	}
@@ -133,6 +139,8 @@ protected function getFilesHooks(array $mockedMethods = [], string $user = 'user
 					$this->notificationGenerator,
 					$this->tagManager,
 					$this->teamManager,
+					$this->fileAccess,
+					$this->mountProviderCollection,
 				])
 				->onlyMethods($mockedMethods)
 				->getMock();
@@ -155,6 +163,8 @@ protected function getFilesHooks(array $mockedMethods = [], string $user = 'user
 			$this->notificationGenerator,
 			$this->tagManager,
 			$this->teamManager,
+			$this->fileAccess,
+			$this->mountProviderCollection,
 		);
 	}
 
@@ -203,6 +213,8 @@ public function testFileCreate(string $currentUser, bool $isPublicShare, string
 				$this->notificationGenerator,
 				$this->tagManager,
 				$this->teamManager,
+				$this->fileAccess,
+				$this->mountProviderCollection,
 			])
 			->onlyMethods(['addNotificationsForFileAction'])
 			->getMock();

tests/psalm-baseline.xml

@@ -1,16 +1,10 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<files psalm-version="6.15.1@28dc127af1b5aecd52314f6f645bafc10d0e11f9">
+<files psalm-version="7.0.0-beta22@de27e5724ee5997a1e3baee4d394ca6f4e46eba0">
   <file src="lib/BackgroundJob/RemoteActivity.php">
     <UndefinedClass>
       <code><![CDATA[ClientException]]></code>
     </UndefinedClass>
   </file>
-  <file src="lib/Controller/RemoteActivityController.php">
-    <UndefinedClass>
-      <code><![CDATA[$storage]]></code>
-      <code><![CDATA[Wrapper]]></code>
-    </UndefinedClass>
-  </file>
   <file src="lib/Data.php">
     <UndefinedDocblockClass>
       <code><![CDATA[$favoriteFilter->filterFavorites($query);]]></code>
@@ -27,15 +21,7 @@
       <code><![CDATA[$this->teamManager]]></code>
       <code><![CDATA[$this->teamManager]]></code>
       <code><![CDATA[Member]]></code>
-      <code><![CDATA[\OCA\GroupFolders\ACL\RuleManager]]></code>
-      <code><![CDATA[\OCA\GroupFolders\Folder\FolderManager]]></code>
       <code><![CDATA[protected]]></code>
     </UndefinedClass>
-    <UndefinedDocblockClass>
-      <code><![CDATA[$rule]]></code>
-      <code><![CDATA[$rule]]></code>
-      <code><![CDATA[$rule]]></code>
-      <code><![CDATA[$rules]]></code>
-    </UndefinedDocblockClass>
   </file>
 </files>

tests/stubs/oc_core_command_base.php

@@ -0,0 +1,49 @@
+<?php
+
+namespace Stecman\Component\Symfony\Console\BashCompletion {
+	class CompletionContext {
+	}
+}
+
+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);
+
+		/**
+		 * @param string $argumentName
+		 * @return array
+		 */
+		public function completeArgumentValues($argumentName, \Stecman\Component\Symfony\Console\BashCompletion\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) {
+		}
+	}
+}