Merge pull request #43865 from nextcloud/backport/43859/stable24

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

@@ -191,7 +191,7 @@ class ServerFactory {
 
 			// Allow view-only plugin for webdav requests
 			$server->addPlugin(new ViewOnlyPlugin(
-				$this->psrLogger
+				$userFolder
 			));
 
 			if ($this->userSession->isLoggedIn()) {

apps/dav/lib/DAV/ViewOnlyPlugin.php

@@ -24,22 +24,24 @@ namespace OCA\DAV\DAV;
 use OCA\DAV\Connector\Sabre\Exception\Forbidden;
 use OCA\DAV\Connector\Sabre\File as DavFile;
 use OCA\Files_Versions\Sabre\VersionFile;
+use OCP\Files\Folder;
 use OCP\Files\NotFoundException;
-use Psr\Log\LoggerInterface;
+use Sabre\DAV\Exception\NotFound;
 use Sabre\DAV\Server;
 use Sabre\DAV\ServerPlugin;
 use Sabre\HTTP\RequestInterface;
-use Sabre\DAV\Exception\NotFound;
 
 /**
  * Sabre plugin for restricting file share receiver download:
  */
 class ViewOnlyPlugin extends ServerPlugin {
 	private ?Server $server = null;
-	private LoggerInterface $logger;
+	private ?Folder $userFolder;
 
-	public function __construct(LoggerInterface $logger) {
-		$this->logger = $logger;
+	public function __construct(
+		?Folder $userFolder
+	) {
+		$this->userFolder = $userFolder;
 	}
 
 	/**
@@ -74,8 +76,18 @@ class ViewOnlyPlugin extends ServerPlugin {
 			if ($davNode instanceof DavFile) {
 				// Restrict view-only to nodes which are shared
 				$node = $davNode->getNode();
-			} else if ($davNode instanceof VersionFile) {
+			} elseif ($davNode instanceof VersionFile) {
 				$node = $davNode->getVersion()->getSourceFile();
+				$currentUserId = $this->userFolder->getOwner()->getUID();
+				// The version source file is relative to the owner storage.
+				// But we need the node from the current user perspective.
+				if ($node->getOwner()->getUID() !== $currentUserId) {
+					$nodes = $this->userFolder->getById($node->getId());
+					$node = array_pop($nodes);
+					if (!$node) {
+						throw new NotFoundException("Version file not accessible by current user");
+					}
+				}
 			} else {
 				return true;
 			}

apps/dav/lib/Server.php

@@ -230,11 +230,6 @@ class Server {
 			$this->server->addPlugin(new FakeLockerPlugin());
 		}
 
-		// Allow view-only plugin for webdav requests
-		$this->server->addPlugin(new ViewOnlyPlugin(
-			\OC::$server->get(LoggerInterface::class)
-		));
-
 		if (BrowserErrorPagePlugin::isBrowserRequest($request)) {
 			$this->server->addPlugin(new BrowserErrorPagePlugin());
 		}
@@ -244,6 +239,11 @@ class Server {
 
 		// wait with registering these until auth is handled and the filesystem is setup
 		$this->server->on('beforeMethod:*', function () use ($root, $lazySearchBackend) {
+			// Allow view-only plugin for webdav requests
+			$this->server->addPlugin(new ViewOnlyPlugin(
+				\OC::$server->getUserFolder(),
+			));
+
 			// custom properties plugin must be the last one
 			$userSession = \OC::$server->getUserSession();
 			$user = $userSession->getUser();

apps/dav/tests/unit/DAV/ViewOnlyPluginTest.php

@@ -26,10 +26,11 @@ use OCA\DAV\Connector\Sabre\File as DavFile;
 use OCA\Files_Versions\Versions\IVersion;
 use OCA\Files_Versions\Sabre\VersionFile;
 use OCP\Files\File;
+use OCP\Files\Folder;
 use OCP\Files\Storage\IStorage;
+use OCP\IUser;
 use OCP\Share\IAttributes;
 use OCP\Share\IShare;
-use Psr\Log\LoggerInterface;
 use Sabre\DAV\Server;
 use Sabre\DAV\Tree;
 use Test\TestCase;
@@ -43,10 +44,13 @@ class ViewOnlyPluginTest extends TestCase {
 	private $tree;
 	/** @var RequestInterface | \PHPUnit\Framework\MockObject\MockObject */
 	private $request;
+	/** @var Folder | \PHPUnit\Framework\MockObject\MockObject */
+	private $userFolder;
 
 	public function setUp(): void {
+		$this->userFolder = $this->createMock(Folder::class);
 		$this->plugin = new ViewOnlyPlugin(
-			$this->createMock(LoggerInterface::class)
+			$this->userFolder,
 		);
 		$this->request = $this->createMock(RequestInterface::class);
 		$this->tree = $this->createMock(Tree::class);
@@ -111,6 +115,26 @@ class ViewOnlyPluginTest extends TestCase {
 			$davNode->expects($this->once())
 				->method('getVersion')
 				->willReturn($version);
+
+			$currentUser = $this->createMock(IUser::class);
+			$currentUser->expects($this->once())
+				->method('getUID')
+				->willReturn('alice');
+			$nodeInfo->expects($this->once())
+				->method('getOwner')
+				->willReturn($currentUser);
+
+			$nodeInfo = $this->createMock(File::class);
+			$owner = $this->createMock(IUser::class);
+			$owner->expects($this->once())
+				->method('getUID')
+				->willReturn('bob');
+			$this->userFolder->expects($this->once())
+				->method('getById')
+				->willReturn([$nodeInfo]);
+			$this->userFolder->expects($this->once())
+				->method('getOwner')
+				->willReturn($owner);
 		} else {
 			$davPath = 'files/path/to/file.odt';
 			$davNode = $this->createMock(DavFile::class);

apps/dav/tests/unit/ServerTest.php

@@ -45,6 +45,7 @@ class ServerTest extends \Test\TestCase {
 		/** @var IRequest | \PHPUnit\Framework\MockObject\MockObject $r */
 		$r = $this->createMock(IRequest::class);
 		$r->expects($this->any())->method('getRequestUri')->willReturn($uri);
+		$this->loginAsUser('admin');
 		$s = new Server($r, '/');
 		$this->assertNotNull($s->server);
 		foreach ($plugins as $plugin) {