fix: Prevent download of view-only files

apps/dav/lib/Connector/Sabre/ZipFolderPlugin.php

@@ -61,9 +61,22 @@ public function initialize(Server $server): void {
 		$this->server->on('afterMethod:GET', $this->afterDownload(...), 999);
 	}
 
+	/**
+	 * Recursively iterate over all nodes in a folder.
+	 */
+	protected function iterateNodes(NcNode $node): iterable {
+		if ($node instanceof NcFile) {
+			yield $node;
+		} elseif ($node instanceof NcFolder) {
+			yield $node;
+			foreach ($node->getDirectoryListing() as $childNode) {
+				yield from $this->iterateNodes($childNode);
+			}
+		}
+	}
+
 	/**
 	 * Adding a node to the archive streamer.
-	 * This will recursively add new nodes to the stream if the node is a directory.
 	 */
 	protected function streamNode(Streamer $streamer, NcNode $node, string $rootPath): void {
 		// Remove the root path from the filename to make it relative to the requested folder
@@ -79,10 +92,6 @@ protected function streamNode(Streamer $streamer, NcNode $node, string $rootPath
 			$streamer->addFileFromStream($resource, $filename, $node->getSize(), $mtime);
 		} elseif ($node instanceof NcFolder) {
 			$streamer->addEmptyDir($filename, $mtime);
-			$content = $node->getDirectoryListing();
-			foreach ($content as $subNode) {
-				$this->streamNode($streamer, $subNode, $rootPath);
-			}
 		}
 	}
 
@@ -137,7 +146,20 @@ public function handleDownload(Request $request, Response $response): ?bool {
 		}
 
 		$folder = $node->getNode();
-		$event = new BeforeZipCreatedEvent($folder, $files);
+		$rootNodes = empty($files) ? $folder->getDirectoryListing() : [];
+		foreach ($files as $path) {
+			$child = $node->getChild($path);
+			assert($child instanceof Node);
+			$rootNodes[] = $child->getNode();
+		}
+		$allNodes = [];
+		foreach ($rootNodes as $rootNode) {
+			foreach ($this->iterateNodes($rootNode) as $node) {
+				$allNodes[] = $node;
+			}
+		}
+
+		$event = new BeforeZipCreatedEvent($folder, $files, $allNodes);
 		$this->eventDispatcher->dispatchTyped($event);
 		if ((!$event->isSuccessful()) || $event->getErrorMessage() !== null) {
 			$errorMessage = $event->getErrorMessage();
@@ -149,13 +171,7 @@ public function handleDownload(Request $request, Response $response): ?bool {
 			// Downloading was denied by an app
 			throw new Forbidden($errorMessage);
 		}
-
-		$content = empty($files) ? $folder->getDirectoryListing() : [];
-		foreach ($files as $path) {
-			$child = $node->getChild($path);
-			assert($child instanceof Node);
-			$content[] = $child->getNode();
-		}
+		$allNodes = $event->getNodes();
 
 		$archiveName = $folder->getName();
 		if (count(explode('/', trim($folder->getPath(), '/'), 3)) === 2) {
@@ -169,13 +185,13 @@ public function handleDownload(Request $request, Response $response): ?bool {
 			$rootPath = dirname($folder->getPath());
 		}
 
-		$streamer = new Streamer($tarRequest, -1, count($content), $this->timezoneFactory);
+		$streamer = new Streamer($tarRequest, -1, count($rootNodes), $this->timezoneFactory);
 		$streamer->sendHeaders($archiveName);
 		// For full folder downloads we also add the folder itself to the archive
 		if (empty($files)) {
 			$streamer->addEmptyDir($archiveName);
 		}
-		foreach ($content as $node) {
+		foreach ($allNodes as $node) {
 			$this->streamNode($streamer, $node, $rootPath);
 		}
 		$streamer->finalize();

apps/files_sharing/lib/Listener/BeforeDirectFileDownloadListener.php

@@ -24,6 +24,7 @@ class BeforeDirectFileDownloadListener implements IEventListener {
 	public function __construct(
 		private IUserSession $userSession,
 		private IRootFolder $rootFolder,
+		private ViewOnly $viewOnly,
 	) {
 	}
 
@@ -32,17 +33,17 @@ public function handle(Event $event): void {
 			return;
 		}
 
-		$pathsToCheck = [$event->getPath()];
-		// Check only for user/group shares. Don't restrict e.g. share links
 		$user = $this->userSession->getUser();
-		if ($user) {
-			$viewOnlyHandler = new ViewOnly(
-				$this->rootFolder->getUserFolder($user->getUID())
-			);
-			if (!$viewOnlyHandler->check($pathsToCheck)) {
-				$event->setSuccessful(false);
-				$event->setErrorMessage('Access to this resource or one of its sub-items has been denied.');
-			}
+		// Check only for user/group shares. Don't restrict e.g. share links
+		if (!$user) {
+			return;
+
+		}
+		$userFolder = $this->rootFolder->getUserFolder($user->getUID());
+		$node = $userFolder->get($event->getPath());
+		if (!$this->viewOnly->isNodeCanBeDownloaded($node)) {
+			$event->setSuccessful(false);
+			$event->setErrorMessage('Access to this resource or one of its sub-items has been denied.');
 		}
 	}
 }

apps/files_sharing/lib/Listener/BeforeZipCreatedListener.php

@@ -24,6 +24,7 @@ class BeforeZipCreatedListener implements IEventListener {
 	public function __construct(
 		private IUserSession $userSession,
 		private IRootFolder $rootFolder,
+		private ViewOnly $viewOnly,
 	) {
 	}
 
@@ -32,28 +33,22 @@ public function handle(Event $event): void {
 			return;
 		}
 
-		$dir = $event->getDirectory();
-		$files = $event->getFiles();
-
-		$pathsToCheck = [];
-		foreach ($files as $file) {
-			$pathsToCheck[] = $dir . '/' . $file;
+		$user = $this->userSession->getUser();
+		if (!$user) {
+			return;
 		}
 
-		// Check only for user/group shares. Don't restrict e.g. share links
-		$user = $this->userSession->getUser();
-		if ($user) {
-			$viewOnlyHandler = new ViewOnly(
-				$this->rootFolder->getUserFolder($user->getUID())
-			);
-			if (!$viewOnlyHandler->check($pathsToCheck)) {
-				$event->setErrorMessage('Access to this resource or one of its sub-items has been denied.');
-				$event->setSuccessful(false);
-			} else {
-				$event->setSuccessful(true);
-			}
-		} else {
-			$event->setSuccessful(true);
+		$userFolder = $this->rootFolder->getUserFolder($user->getUID());
+		// Check whether the user can download the requested folder
+		$folder = $userFolder->get(substr($event->getDirectory(), strlen($userFolder->getPath())));
+		if (!$this->viewOnly->isNodeCanBeDownloaded($folder)) {
+			$event->setSuccessful(false);
+			$event->setErrorMessage('Access to this resource has been denied.');
+			return;
 		}
+
+		$nodes = array_filter($event->getNodes(), fn ($node) => $this->viewOnly->isNodeCanBeDownloaded($node));
+		$event->setNodes(array_values($nodes));
+		$event->setSuccessful(true);
 	}
 }

apps/files_sharing/lib/ViewOnly.php

@@ -8,80 +8,15 @@
 
 namespace OCA\Files_Sharing;
 
-use OCP\Files\File;
-use OCP\Files\Folder;
 use OCP\Files\Node;
-use OCP\Files\NotFoundException;
 
 /**
  * Handles restricting for download of files
  */
 class ViewOnly {
-
-	public function __construct(
-		private Folder $userFolder,
-	) {
-	}
-
-	/**
-	 * @param string[] $pathsToCheck
-	 * @return bool
-	 */
-	public function check(array $pathsToCheck): bool {
-		// If any of elements cannot be downloaded, prevent whole download
-		foreach ($pathsToCheck as $file) {
-			try {
-				$info = $this->userFolder->get($file);
-				if ($info instanceof File) {
-					// access to filecache is expensive in the loop
-					if (!$this->checkFileInfo($info)) {
-						return false;
-					}
-				} elseif ($info instanceof Folder) {
-					// get directory content is rather cheap query
-					if (!$this->dirRecursiveCheck($info)) {
-						return false;
-					}
-				}
-			} catch (NotFoundException $e) {
-				continue;
-			}
-		}
-		return true;
-	}
-
-	/**
-	 * @param Folder $dirInfo
-	 * @return bool
-	 * @throws NotFoundException
-	 */
-	private function dirRecursiveCheck(Folder $dirInfo): bool {
-		if (!$this->checkFileInfo($dirInfo)) {
-			return false;
-		}
-		// If any of elements cannot be downloaded, prevent whole download
-		$files = $dirInfo->getDirectoryListing();
-		foreach ($files as $file) {
-			if ($file instanceof File) {
-				if (!$this->checkFileInfo($file)) {
-					return false;
-				}
-			} elseif ($file instanceof Folder) {
-				return $this->dirRecursiveCheck($file);
-			}
-		}
-
-		return true;
-	}
-
-	/**
-	 * @param Node $fileInfo
-	 * @return bool
-	 * @throws NotFoundException
-	 */
-	private function checkFileInfo(Node $fileInfo): bool {
+	public function isNodeCanBeDownloaded(Node $node): bool {
 		// Restrict view-only to nodes which are shared
-		$storage = $fileInfo->getStorage();
+		$storage = $node->getStorage();
 		if (!$storage->instanceOfStorage(SharedStorage::class)) {
 			return true;
 		}

lib/public/Files/Events/BeforeZipCreatedEvent.php

@@ -11,6 +11,7 @@
 
 use OCP\EventDispatcher\Event;
 use OCP\Files\Folder;
+use OCP\Files\Node;
 
 /**
  * This event is triggered before a archive is created when a user requested
@@ -27,13 +28,15 @@ class BeforeZipCreatedEvent extends Event {
 	private ?Folder $folder = null;
 
 	/**
-	 * @param list<string> $files
+	 * @param list<string> $files Selected files, empty for folder selection
+	 * @param list<Node> $nodes Recursively collected nodes
 	 * @since 25.0.0
 	 * @since 31.0.0 support `OCP\Files\Folder` as `$directory` parameter - passing a string is deprecated now
 	 */
 	public function __construct(
 		string|Folder $directory,
 		private array $files,
+		private array $nodes = [],
 	) {
 		parent::__construct();
 		if ($directory instanceof Folder) {
@@ -65,6 +68,20 @@ public function getFiles(): array {
 		return $this->files;
 	}
 
+	/**
+	 * @return Node[]
+	 */
+	public function getNodes(): array {
+		return $this->nodes;
+	}
+
+	/**
+	 * @param Node[] $nodes
+	 */
+	public function setNodes(array $nodes): void {
+		$this->nodes = $nodes;
+	}
+
 	/**
 	 * @since 25.0.0
 	 */