fix: make canManageACL work with circles

Signed-off-by: Robin Appelman <[email protected]>
nextcloud · Feb 6, 2025 · 6f3f5bd · 6f3f5bd
1 parent 4f3bef3
commit 6f3f5bd
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 21 deletions.
diff --git a/lib/ACL/UserMapping/IUserMapping.php b/lib/ACL/UserMapping/IUserMapping.php
@@ -15,4 +15,6 @@ public function getType(): string;
 	public function getId(): string;
 
 	public function getDisplayName(): string;
+
+	public function getKey(): string;
 }
diff --git a/lib/ACL/UserMapping/IUserMappingManager.php b/lib/ACL/UserMapping/IUserMappingManager.php
@@ -24,4 +24,13 @@ public function getMappingsForUser(IUser $user, bool $userAssignable = true): ar
 	 * @return IUserMapping|null
 	 */
 	public function mappingFromId(string $type, string $id): ?IUserMapping;
+
+	/**
+	 * Check if a user is a member of one of the provided user mappings
+	 *
+	 * @param IUser $user
+	 * @param IUserMapping[] $mappings
+	 * @return bool
+	 */
+	public function userInMappings(IUser $user, array $mappings): bool;
 }
diff --git a/lib/ACL/UserMapping/UserMapping.php b/lib/ACL/UserMapping/UserMapping.php
@@ -34,4 +34,8 @@ public function getId(): string {
 	public function getDisplayName(): string {
 		return $this->displayName;
 	}
+
+	public function getKey(): string {
+		return $this->getType() . ':' . $this->getId();
+	}
 }
diff --git a/lib/ACL/UserMapping/UserMappingManager.php b/lib/ACL/UserMapping/UserMappingManager.php
@@ -116,4 +116,22 @@ public function getCirclesManager(): ?CirclesManager {
 			return null;
 		}
 	}
+
+	public function userInMappings(IUser $user, array $mappings): bool {
+		foreach ($mappings as $mapping) {
+			if ($mapping->getType() === 'user' && $mapping->getId() === $user->getUID()) {
+				return true;
+			}
+		}
+
+		$mappingKeys = array_map(fn (IUserMapping $mapping) => $mapping->getKey(), $mappings);
+
+		$userMappings = $this->getMappingsForUser($user);
+		foreach ($userMappings as $userMapping) {
+			if (in_array($userMapping->getKey(), $mappingKeys, true)) {
+				return true;
+			}
+		}
+		return false;
+	}
 }
diff --git a/lib/Folder/FolderManager.php b/lib/Folder/FolderManager.php
@@ -13,6 +13,9 @@
 use OCA\Circles\Model\Circle;
 use OCA\Circles\Model\Member;
 use OCA\Circles\Model\Probes\CircleProbe;
+use OCA\GroupFolders\ACL\UserMapping\IUserMapping;
+use OCA\GroupFolders\ACL\UserMapping\IUserMappingManager;
+use OCA\GroupFolders\ACL\UserMapping\UserMapping;
 use OCA\GroupFolders\Mount\GroupMountPoint;
 use OCA\GroupFolders\ResponseDefinitions;
 use OCP\AutoloadNotAllowedException;
@@ -75,6 +78,7 @@ public function __construct(
 		private LoggerInterface $logger,
 		private IEventDispatcher $eventDispatcher,
 		private IConfig $config,
+		private IUserMappingManager $userMappingManager,
 	) {
 	}
 
@@ -454,28 +458,26 @@ public function canManageACL(int $folderId, IUser $user): bool {
 			}
 		}
 
-		$query = $this->connection->getQueryBuilder();
-		$query->select('*')
-			->from('group_folders_manage')
-			->where($query->expr()->eq('folder_id', $query->createNamedParameter($folderId, IQueryBuilder::PARAM_INT)))
-			->andWhere($query->expr()->eq('mapping_type', $query->createNamedParameter('user')))
-			->andWhere($query->expr()->eq('mapping_id', $query->createNamedParameter($userId)));
-		if ($query->executeQuery()->rowCount() === 1) {
-			return true;
-		}
+		$managerMappings = $this->getManagerMappings($folderId);
+		return $this->userMappingManager->userInMappings($user, $managerMappings);
+	}
 
+	/**
+	 * @param int $folderId
+	 * @return IUserMapping[]
+	 */
+	private function getManagerMappings(int $folderId): array {
 		$query = $this->connection->getQueryBuilder();
-		$query->select('*')
+		$query->select('mapping_type', 'mapping_id')
 			->from('group_folders_manage')
-			->where($query->expr()->eq('folder_id', $query->createNamedParameter($folderId)))
-			->andWhere($query->expr()->eq('mapping_type', $query->createNamedParameter('group')));
-		$groups = $query->executeQuery()->fetchAll();
-		foreach ($groups as $manageRule) {
-			if ($this->groupManager->isInGroup($userId, $manageRule['mapping_id'])) {
-				return true;
-			}
+			->where($query->expr()->eq('folder_id', $query->createNamedParameter($folderId, IQueryBuilder::PARAM_INT)));
+		$managerMappings = [];
+
+		$rows = $query->executeQuery()->fetchAll();
+		foreach ($rows as $manageRule) {
+			$managerMappings[] = new UserMapping($manageRule['mapping_type'], $manageRule['mapping_id']);
 		}
-		return false;
+		return $managerMappings;
 	}
 
 	/**

diff --git a/tests/Folder/FolderManagerTest.php b/tests/Folder/FolderManagerTest.php
@@ -6,6 +6,7 @@
 
 namespace OCA\GroupFolders\Tests\Folder;
 
+use OCA\GroupFolders\ACL\UserMapping\IUserMappingManager;
 use OCA\GroupFolders\Folder\FolderManager;
 use OCP\Constants;
 use OCP\EventDispatcher\IEventDispatcher;
@@ -28,6 +29,7 @@ class FolderManagerTest extends TestCase {
 	private LoggerInterface $logger;
 	private IEventDispatcher $eventDispatcher;
 	private IConfig $config;
+	private IUserMappingManager $userMappingManager;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -37,13 +39,15 @@ protected function setUp(): void {
 		$this->logger = $this->createMock(LoggerInterface::class);
 		$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
 		$this->config = $this->createMock(IConfig::class);
+		$this->userMappingManager = $this->createMock(IUserMappingManager::class);
 		$this->manager = new FolderManager(
 			\OC::$server->getDatabaseConnection(),
 			$this->groupManager,
 			$this->mimeLoader,
 			$this->logger,
 			$this->eventDispatcher,
 			$this->config,
+			$this->userMappingManager,
 		);
 		$this->clean();
 	}
@@ -363,7 +367,7 @@ public function testGetFoldersForUserSimple() {
 		$db = $this->createMock(IDBConnection::class);
 		/** @var FolderManager|\PHPUnit_Framework_MockObject_MockObject $manager */
 		$manager = $this->getMockBuilder(FolderManager::class)
-			->setConstructorArgs([$db, $this->groupManager, $this->mimeLoader, $this->logger, $this->eventDispatcher, $this->config])
+			->setConstructorArgs([$db, $this->groupManager, $this->mimeLoader, $this->logger, $this->eventDispatcher, $this->config, $this->userMappingManager])
 			->setMethods(['getFoldersForGroups'])
 			->getMock();
 
@@ -386,7 +390,7 @@ public function testGetFoldersForUserMerge() {
 		$db = $this->createMock(IDBConnection::class);
 		/** @var FolderManager|\PHPUnit_Framework_MockObject_MockObject $manager */
 		$manager = $this->getMockBuilder(FolderManager::class)
-			->setConstructorArgs([$db, $this->groupManager, $this->mimeLoader, $this->logger, $this->eventDispatcher, $this->config])
+			->setConstructorArgs([$db, $this->groupManager, $this->mimeLoader, $this->logger, $this->eventDispatcher, $this->config, $this->userMappingManager])
 			->setMethods(['getFoldersForGroups'])
 			->getMock();
 
@@ -422,7 +426,7 @@ public function testGetFolderPermissionsForUserMerge() {
 		$db = $this->createMock(IDBConnection::class);
 		/** @var FolderManager|\PHPUnit_Framework_MockObject_MockObject $manager */
 		$manager = $this->getMockBuilder(FolderManager::class)
-			->setConstructorArgs([$db, $this->groupManager, $this->mimeLoader, $this->logger, $this->eventDispatcher, $this->config])
+			->setConstructorArgs([$db, $this->groupManager, $this->mimeLoader, $this->logger, $this->eventDispatcher, $this->config, $this->userMappingManager])
 			->setMethods(['getFoldersForGroups'])
 			->getMock();