Merge pull request #44326 from nextcloud/backport/44309/stable23

[stable23] Forbid tagging readonly files
nextcloud · Mar 26, 2024 · d8ea28b · d8ea28b
2 parents 3795d7e + a4473ce
commit d8ea28b
Show file tree

Hide file tree

Showing 7 changed files with 160 additions and 189 deletions.
diff --git a/apps/dav/lib/SystemTag/SystemTagMappingNode.php b/apps/dav/lib/SystemTag/SystemTagMappingNode.php
@@ -37,62 +37,37 @@
  * Mapping node for system tag to object id
  */
 class SystemTagMappingNode implements \Sabre\DAV\INode {
-	/**
-	 * @var ISystemTag
-	 */
-	protected $tag;
-
-	/**
-	 * @var string
-	 */
+	/** @var ISystemTag */
+	private $tag;
+	/** @var string */
 	private $objectId;
-
-	/**
-	 * @var string
-	 */
+	/** @var string */
 	private $objectType;
-
-	/**
-	 * User
-	 *
-	 * @var IUser
-	 */
-	protected $user;
-
-	/**
-	 * @var ISystemTagManager
-	 */
-	protected $tagManager;
-
-	/**
-	 * @var ISystemTagObjectMapper
-	 */
+	/** @var IUser */
+	private $user;
+	/** @var ISystemTagManager */
+	private $tagManager;
+	/** @var ISystemTagObjectMapper */
 	private $tagMapper;
+	/** @var \Closure */
+	private $childWriteAccessFunction;
 
-	/**
-	 * Sets up the node, expects a full path name
-	 *
-	 * @param ISystemTag $tag system tag
-	 * @param string $objectId
-	 * @param string $objectType
-	 * @param IUser $user user
-	 * @param ISystemTagManager $tagManager
-	 * @param ISystemTagObjectMapper $tagMapper
-	 */
 	public function __construct(
 		ISystemTag $tag,
-		$objectId,
-		$objectType,
+		string $objectId,
+		string $objectType,
 		IUser $user,
 		ISystemTagManager $tagManager,
-		ISystemTagObjectMapper $tagMapper
+		ISystemTagObjectMapper $tagMapper,
+		\Closure $childWriteAccessFunction
 	) {
 		$this->tag = $tag;
 		$this->objectId = $objectId;
 		$this->objectType = $objectType;
 		$this->user = $user;
 		$this->tagManager = $tagManager;
 		$this->tagMapper = $tagMapper;
+		$this->childWriteAccessFunction = $childWriteAccessFunction;
 	}
 
 	/**
@@ -161,6 +136,10 @@ public function delete() {
 			if (!$this->tagManager->canUserAssignTag($this->tag, $this->user)) {
 				throw new Forbidden('No permission to unassign tag ' . $this->tag->getId());
 			}
+			$writeAccessFunction = $this->childWriteAccessFunction;
+			if (!$writeAccessFunction($this->objectId)) {
+				throw new Forbidden('No permission to unassign tag to ' . $this->objectId);
+			}
 			$this->tagMapper->unassignTags($this->objectId, $this->objectType, $this->tag->getId());
 		} catch (TagNotFoundException $e) {
 			// can happen if concurrent deletion occurred

diff --git a/apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php b/apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php
@@ -40,56 +40,33 @@
  * Collection containing tags by object id
  */
 class SystemTagsObjectMappingCollection implements ICollection {
-
-	/**
-	 * @var string
-	 */
+	/** @var string */
 	private $objectId;
-
-	/**
-	 * @var string
-	 */
+	/** @var string */
 	private $objectType;
-
-	/**
-	 * @var ISystemTagManager
-	 */
+	/** @var IUser */
+	private $user;
+	/** @var ISystemTagManager */
 	private $tagManager;
-
-	/**
-	 * @var ISystemTagObjectMapper
-	 */
+	/** @var ISystemTagObjectMapper */
 	private $tagMapper;
+	/** @var \Closure */
+	protected $childWriteAccessFunction;
 
-	/**
-	 * User
-	 *
-	 * @var IUser
-	 */
-	private $user;
-
-
-	/**
-	 * Constructor
-	 *
-	 * @param string $objectId object id
-	 * @param string $objectType object type
-	 * @param IUser $user user
-	 * @param ISystemTagManager $tagManager tag manager
-	 * @param ISystemTagObjectMapper $tagMapper tag mapper
-	 */
 	public function __construct(
-		$objectId,
-		$objectType,
+		string $objectId,
+		string $objectType,
 		IUser $user,
 		ISystemTagManager $tagManager,
-		ISystemTagObjectMapper $tagMapper
+		ISystemTagObjectMapper $tagMapper,
+		\Closure $childWriteAccessFunction
 	) {
-		$this->tagManager = $tagManager;
-		$this->tagMapper = $tagMapper;
 		$this->objectId = $objectId;
 		$this->objectType = $objectType;
 		$this->user = $user;
+		$this->tagManager = $tagManager;
+		$this->tagMapper = $tagMapper;
+		$this->childWriteAccessFunction = $childWriteAccessFunction;
 	}
 
 	public function createFile($name, $data = null) {
@@ -103,7 +80,10 @@ public function createFile($name, $data = null) {
 			if (!$this->tagManager->canUserAssignTag($tag, $this->user)) {
 				throw new Forbidden('No permission to assign tag ' . $tagId);
 			}
-
+			$writeAccessFunction = $this->childWriteAccessFunction;
+			if (!$writeAccessFunction($this->objectId)) {
+				throw new Forbidden('No permission to assign tag to ' . $this->objectId);
+			}
 			$this->tagMapper->assignTags($this->objectId, $this->objectType, $tagId);
 		} catch (TagNotFoundException $e) {
 			throw new PreconditionFailed('Tag with id ' . $tagId . ' does not exist, cannot assign');
@@ -204,7 +184,8 @@ private function makeNode(ISystemTag $tag) {
 			$this->objectType,
 			$this->user,
 			$this->tagManager,
-			$this->tagMapper
+			$this->tagMapper,
+			$this->childWriteAccessFunction,
 		);
 	}
 }
diff --git a/apps/dav/lib/SystemTag/SystemTagsObjectTypeCollection.php b/apps/dav/lib/SystemTag/SystemTagsObjectTypeCollection.php
@@ -38,61 +38,37 @@
  * Collection containing object ids by object type
  */
 class SystemTagsObjectTypeCollection implements ICollection {
-
-	/**
-	 * @var string
-	 */
+	/** @var string */
 	private $objectType;
-
-	/**
-	 * @var ISystemTagManager
-	 */
+	/** @var ISystemTagManager */
 	private $tagManager;
-
-	/**
-	 * @var ISystemTagObjectMapper
-	 */
+	/** @var ISystemTagObjectMapper */
 	private $tagMapper;
-
-	/**
-	 * @var IGroupManager
-	 */
-	private $groupManager;
-
-	/**
-	 * @var IUserSession
-	 */
+	/** @var IUserSession */
 	private $userSession;
-
-	/**
-	 * @var \Closure
-	 **/
+	/** @var IGroupManager */
+	private $groupManager;
+	/** @var \Closure */
 	protected $childExistsFunction;
+	/** @var \Closure */
+	protected $childWriteAccessFunction;
 
-	/**
-	 * Constructor
-	 *
-	 * @param string $objectType object type
-	 * @param ISystemTagManager $tagManager
-	 * @param ISystemTagObjectMapper $tagMapper
-	 * @param IUserSession $userSession
-	 * @param IGroupManager $groupManager
-	 * @param \Closure $childExistsFunction
-	 */
 	public function __construct(
-		$objectType,
+		string $objectType,
 		ISystemTagManager $tagManager,
 		ISystemTagObjectMapper $tagMapper,
 		IUserSession $userSession,
 		IGroupManager $groupManager,
-		\Closure $childExistsFunction
+		\Closure $childExistsFunction,
+		\Closure $childWriteAccessFunction
 	) {
+		$this->objectType = $objectType;
 		$this->tagManager = $tagManager;
 		$this->tagMapper = $tagMapper;
-		$this->objectType = $objectType;
 		$this->userSession = $userSession;
 		$this->groupManager = $groupManager;
 		$this->childExistsFunction = $childExistsFunction;
+		$this->childWriteAccessFunction = $childWriteAccessFunction;
 	}
 
 	/**
@@ -129,7 +105,8 @@ public function getChild($objectName) {
 			$this->objectType,
 			$this->userSession->getUser(),
 			$this->tagManager,
-			$this->tagMapper
+			$this->tagMapper,
+			$this->childWriteAccessFunction,
 		);
 	}
 

diff --git a/apps/dav/lib/SystemTag/SystemTagsRelationsCollection.php b/apps/dav/lib/SystemTag/SystemTagsRelationsCollection.php
@@ -26,6 +26,7 @@
  */
 namespace OCA\DAV\SystemTag;
 
+use OCP\Constants;
 use OCP\IGroupManager;
 use OCP\IUserSession;
 use OCP\SystemTag\ISystemTagManager;
@@ -36,7 +37,6 @@
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 
 class SystemTagsRelationsCollection extends SimpleCollection {
-
 	/**
 	 * SystemTagsRelationsCollection constructor.
 	 *
@@ -60,10 +60,19 @@ public function __construct(
 				$tagMapper,
 				$userSession,
 				$groupManager,
-				function ($name) {
+				function ($name): bool {
 					$nodes = \OC::$server->getUserFolder()->getById((int)$name);
 					return !empty($nodes);
-				}
+				},
+				function ($name): bool {
+					$nodes = \OC::$server->getUserFolder()->getById((int)$name);
+					foreach ($nodes as $node) {
+						if (($node->getPermissions() & Constants::PERMISSION_UPDATE) === Constants::PERMISSION_UPDATE) {
+							return true;
+						}
+					}
+					return false;
+				},
 			),
 		];
 
@@ -77,7 +86,10 @@ function ($name) {
 				$tagMapper,
 				$userSession,
 				$groupManager,
-				$entityExistsFunction
+				$entityExistsFunction,
+				function ($name) {
+					return true;
+				},
 			);
 		}