Merge pull request #51682 from nextcloud/fix/tag-fileid-check

fix(dav): filter user files when updating tags

Author: skjnldsv

apps/dav/lib/SystemTag/SystemTagPlugin.php

@@ -11,6 +11,8 @@
 use OCA\DAV\Connector\Sabre\FilesPlugin;
 use OCA\DAV\Connector\Sabre\Node;
 use OCP\AppFramework\Http;
+use OCP\Constants;
+use OCP\Files\IRootFolder;
 use OCP\IGroupManager;
 use OCP\IUser;
 use OCP\IUserSession;
@@ -68,7 +70,8 @@ public function __construct(
 		protected ISystemTagManager $tagManager,
 		protected IGroupManager $groupManager,
 		protected IUserSession $userSession,
-		private ISystemTagObjectMapper $tagMapper,
+		protected IRootFolder $rootFolder,
+		protected ISystemTagObjectMapper $tagMapper,
 	) {
 	}
 
@@ -387,6 +390,11 @@ public function handleUpdateProperties($path, PropPatch $propPatch) {
 			}
 
 			if (isset($props[self::OBJECTIDS_PROPERTYNAME])) {
+				$user = $this->userSession->getUser();
+				if (!$user) {
+					throw new Forbidden('You don’t have permissions to update tags');
+				}
+
 				$propValue = $props[self::OBJECTIDS_PROPERTYNAME];
 				if (!$propValue instanceof SystemTagsObjectList || count($propValue->getObjects()) === 0) {
 					throw new BadRequest('Invalid object-ids property');
@@ -399,10 +407,35 @@ public function handleUpdateProperties($path, PropPatch $propPatch) {
 					throw new BadRequest('Invalid object-ids property. All object types must be of the same type: ' . $node->getName());
 				}
 
+				// Only files are supported at the moment
+				// Also see SystemTagsRelationsCollection file
+				if ($objectTypes[0] !== 'files') {
+					throw new BadRequest('Invalid object-ids property type. Only files are supported');
+				}
+
+				// Get all current tagged objects
+				$taggedObjects = $this->tagMapper->getObjectIdsForTags([$node->getSystemTag()->getId()], 'files');
+				$toAddObjects = array_map(fn ($value) => (string)$value, array_keys($objects));
+
+				// Compute the tags to add and remove
+				$addedObjects = array_values(array_diff($toAddObjects, $taggedObjects));
+				$removedObjects = array_values(array_diff($taggedObjects, $toAddObjects));
+
+				// Check permissions for each object to be freshly tagged or untagged
+				if (!$this->canUpdateTagForFileIds(array_merge($addedObjects, $removedObjects))) {
+					throw new Forbidden('You don’t have permissions to update tags');
+				}
+
 				$this->tagMapper->setObjectIdsForTag($node->getSystemTag()->getId(), $node->getName(), array_keys($objects));
 			}
 
 			if ($props[self::OBJECTIDS_PROPERTYNAME] === null) {
+				// Check the user have permissions to remove the tag from all currently tagged objects
+				$taggedObjects = $this->tagMapper->getObjectIdsForTags([$node->getSystemTag()->getId()], 'files');
+				if (!$this->canUpdateTagForFileIds($taggedObjects)) {
+					throw new Forbidden('You don’t have permissions to update tags');
+				}
+
 				$this->tagMapper->setObjectIdsForTag($node->getSystemTag()->getId(), $node->getName(), []);
 			}
 
@@ -483,4 +516,24 @@ public function handleUpdateProperties($path, PropPatch $propPatch) {
 			return true;
 		});
 	}
+
+	/**
+	 * Check if the user can update the tag for the given file ids
+	 *
+	 * @param list<string> $fileIds
+	 * @return bool
+	 */
+	private function canUpdateTagForFileIds(array $fileIds): bool {
+		$user = $this->userSession->getUser();
+		$userFolder = $this->rootFolder->getUserFolder($user->getUID());
+		foreach ($fileIds as $fileId) {
+			$nodes = $userFolder->getById((int)$fileId);
+			foreach ($nodes as $node) {
+				if (($node->getPermissions() & Constants::PERMISSION_UPDATE) === Constants::PERMISSION_UPDATE) {
+					return true;
+				}
+			}
+		}
+		return false;
+	}
 }

apps/dav/lib/SystemTag/SystemTagsObjectList.php

@@ -31,6 +31,11 @@ public function __construct(
 	) {
 	}
 
+	/**
+	 * Get the object ids and their types.
+	 *
+	 * @return array<string, string>
+	 */
 	public function getObjects(): array {
 		return $this->objects;
 	}
@@ -55,7 +60,7 @@ public static function xmlDeserialize(Reader $reader) {
 					}
 				}
 				if ($id !== '' && $type !== '') {
-					$objects[$id] = $type;
+					$objects[(string)$id] = (string)$type;
 				}
 			}
 		}

apps/dav/lib/SystemTag/SystemTagsRelationsCollection.php

@@ -28,6 +28,8 @@ public function __construct(
 		IRootFolder $rootFolder,
 	) {
 		$children = [
+			// Only files are supported at the moment
+			// Also see SystemTagPlugin::OBJECTIDS_PROPERTYNAME supported types
 			new SystemTagsObjectTypeCollection(
 				'files',
 				$tagManager,

apps/dav/tests/unit/SystemTag/SystemTagPluginTest.php

@@ -12,6 +12,7 @@
 use OCA\DAV\SystemTag\SystemTagPlugin;
 use OCA\DAV\SystemTag\SystemTagsByIdCollection;
 use OCA\DAV\SystemTag\SystemTagsObjectMappingCollection;
+use OCP\Files\IRootFolder;
 use OCP\IGroupManager;
 use OCP\IUser;
 use OCP\IUserSession;
@@ -56,6 +57,11 @@ class SystemTagPluginTest extends \Test\TestCase {
 	 */
 	private $userSession;
 
+	/**
+	 * @var IRootFolder
+	 */
+	private $rootFolder;
+
 	/**
 	 * @var IUser
 	 */
@@ -95,13 +101,17 @@ protected function setUp(): void {
 			->expects($this->any())
 			->method('isLoggedIn')
 			->willReturn(true);
+
 		$this->tagMapper = $this->getMockBuilder(ISystemTagObjectMapper::class)
 			->getMock();
+		$this->rootFolder = $this->getMockBuilder(IRootFolder::class)
+			->getMock();
 
 		$this->plugin = new SystemTagPlugin(
 			$this->tagManager,
 			$this->groupManager,
 			$this->userSession,
+			$this->rootFolder,
 			$this->tagMapper
 		);
 		$this->plugin->initialize($this->server);