fix: Prevent download of view-only files (alternative approach)

Signed-off-by: Kostiantyn Miakshyn <molodchick@gmail.com>

Author: Koc

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

@@ -61,9 +61,21 @@ 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) {
+			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 +91,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,14 +145,20 @@ public function handleDownload(Request $request, Response $response): ?bool {
 		}
 
 		$folder = $node->getNode();
-		$nodes = empty($files) ? $folder->getDirectoryListing() : [];
+		$rootNodes = empty($files) ? $folder->getDirectoryListing() : [];
 		foreach ($files as $path) {
 			$child = $node->getChild($path);
 			assert($child instanceof Node);
-			$nodes[] = $child->getNode();
+			$rootNodes[] = $child->getNode();
+		}
+		$allNodes = [];
+		foreach ($rootNodes as $rootNode) {
+			foreach ($this->iterateNodes($rootNode) as $node) {
+				$allNodes[] = $node;
+			}
 		}
 
-		$event = new BeforeZipCreatedEvent($folder, $files, $nodes);
+		$event = new BeforeZipCreatedEvent($folder, $files, $allNodes);
 		$this->eventDispatcher->dispatchTyped($event);
 		if ((!$event->isSuccessful()) || $event->getErrorMessage() !== null) {
 			$errorMessage = $event->getErrorMessage();
@@ -156,8 +170,7 @@ public function handleDownload(Request $request, Response $response): ?bool {
 			// Downloading was denied by an app
 			throw new Forbidden($errorMessage);
 		}
-
-		$nodes = $event->getNodes();
+		$allNodes = $event->getNodes();
 
 		$archiveName = $folder->getName();
 		if (count(explode('/', trim($folder->getPath(), '/'), 3)) === 2) {
@@ -171,13 +184,13 @@ public function handleDownload(Request $request, Response $response): ?bool {
 			$rootPath = dirname($folder->getPath());
 		}
 
-		$streamer = new Streamer($tarRequest, -1, count($nodes), $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 ($nodes as $node) {
+		foreach ($allNodes as $node) {
 			$this->streamNode($streamer, $node, $rootPath);
 		}
 		$streamer->finalize();

apps/files_sharing/lib/Listener/BeforeZipCreatedListener.php

@@ -9,11 +9,12 @@
 
 namespace OCA\Files_Sharing\Listener;
 
-use OCA\Files_Sharing\ViewOnly;
+use OCA\Files_Sharing\SharedStorage;
 use OCP\EventDispatcher\Event;
 use OCP\EventDispatcher\IEventListener;
 use OCP\Files\Events\BeforeZipCreatedEvent;
 use OCP\Files\IRootFolder;
+use OCP\Files\Node;
 use OCP\IUserSession;
 
 /**
@@ -36,34 +37,39 @@ public function handle(Event $event): void {
 		if (!$user) {
 			// fixme: do we need check anything for anonymous users?
 			$event->setSuccessful(true);
+
+			return;
 		}
 
 		$userFolder = $this->rootFolder->getUserFolder($user->getUID());
-		$viewOnlyHandler = new ViewOnly($userFolder);
-		$this->checkSelectedFilesCanBeDownloaded($event, $viewOnlyHandler);
-		$this->checkNodesCanBeDownloaded($event, $viewOnlyHandler);
+		// Check whether the user can download requested folder
+		$folder = $userFolder->get(substr($event->getDirectory(), strlen($userFolder->getPath())));
+		if (!$this->isNodeCanBeDownloaded($folder)) {
+			$event->setSuccessful(false);
+			$event->setErrorMessage('Access to this resource has been denied.');
+			return;
+		}
+
+		$nodes = array_filter($event->getNodes(), fn($node) => $this->isNodeCanBeDownloaded($node));
+		$event->setNodes(array_values($nodes));
+		$event->setSuccessful(true);
 	}
 
-	private function checkSelectedFilesCanBeDownloaded(BeforeZipCreatedEvent $event, ViewOnly $viewOnlyHandler): void {
-		// Check only for user/group shares. Don't restrict e.g. share links
-		$dir = $event->getDirectory();
-		$pathsToCheck = [$dir];
-		foreach ($event->getFiles() as $file) {
-			$pathsToCheck[] = $dir . '/' . $file;
+	public function isNodeCanBeDownloaded(Node $node): bool {
+		// Restrict view-only to nodes which are shared
+		$storage = $node->getStorage();
+		if (!$storage->instanceOfStorage(SharedStorage::class)) {
+			return true;
 		}
 
-		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);
-		}
-	}
+		// Extract extra permissions
+		/** @var SharedStorage $storage */
+		$share = $storage->getShare();
 
-	private function checkNodesCanBeDownloaded(BeforeZipCreatedEvent $event, ViewOnly $viewOnlyHandler): void {
-		$nodes = array_values(array_filter($event->getNodes(),
-			static fn ($node) => $viewOnlyHandler->checkNode($node)));
+		// Check whether download-permission was denied (granted if not set)
+		$attributes = $share->getAttributes();
+		$canDownload = $attributes?->getAttribute('permissions', 'download');
 
-		$event->setNodes($nodes);
+		return $canDownload !== false;
 	}
 }

apps/files_sharing/lib/ViewOnly.php

@@ -28,8 +28,8 @@ class BeforeZipCreatedEvent extends Event {
 	private ?Folder $folder = null;
 
 	/**
-	 * @param list<string> $files
-	 * @param list<Node> $nodes
+	 * @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
 	 */