From 8e95d0f3ae4fb9e186e3995cd2aa7d8393d7003d Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Tue, 13 Feb 2024 14:46:04 +0100 Subject: [PATCH] Check share attributes when downloading versions Signed-off-by: Louis Chemineau --- .../dav/lib/Connector/Sabre/ServerFactory.php | 2 +- apps/dav/lib/DAV/ViewOnlyPlugin.php | 20 ++++++++++--- apps/dav/lib/Server.php | 2 +- .../dav/tests/unit/DAV/ViewOnlyPluginTest.php | 28 +++++++++++++++++-- apps/dav/tests/unit/ServerTest.php | 1 + 5 files changed, 45 insertions(+), 8 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php index 828977fd812..113cd8a8c23 100644 --- a/apps/dav/lib/Connector/Sabre/ServerFactory.php +++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php @@ -161,7 +161,7 @@ class ServerFactory { // Allow view-only plugin for webdav requests $server->addPlugin(new ViewOnlyPlugin( - $this->logger + $userFolder, )); if ($this->userSession->isLoggedIn()) { diff --git a/apps/dav/lib/DAV/ViewOnlyPlugin.php b/apps/dav/lib/DAV/ViewOnlyPlugin.php index 77a9acd628e..389dd96efb4 100644 --- a/apps/dav/lib/DAV/ViewOnlyPlugin.php +++ b/apps/dav/lib/DAV/ViewOnlyPlugin.php @@ -24,8 +24,8 @@ 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; @@ -36,10 +36,12 @@ use Sabre\HTTP\RequestInterface; */ 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; } /** @@ -76,6 +78,16 @@ class ViewOnlyPlugin extends ServerPlugin { $node = $davNode->getNode(); } 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; } diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index dedb959c1cd..3197476437b 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -240,7 +240,7 @@ class Server { // Allow view-only plugin for webdav requests $this->server->addPlugin(new ViewOnlyPlugin( - $logger + \OC::$server->getUserFolder(), )); if (BrowserErrorPagePlugin::isBrowserRequest($request)) { diff --git a/apps/dav/tests/unit/DAV/ViewOnlyPluginTest.php b/apps/dav/tests/unit/DAV/ViewOnlyPluginTest.php index 00dde60d234..32fd9f452b5 100644 --- a/apps/dav/tests/unit/DAV/ViewOnlyPluginTest.php +++ b/apps/dav/tests/unit/DAV/ViewOnlyPluginTest.php @@ -27,10 +27,11 @@ use OCA\Files_Sharing\SharedStorage; use OCA\Files_Versions\Sabre\VersionFile; use OCA\Files_Versions\Versions\IVersion; 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 Sabre\HTTP\RequestInterface; @@ -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); diff --git a/apps/dav/tests/unit/ServerTest.php b/apps/dav/tests/unit/ServerTest.php index 62e2accd697..26309d5fcd4 100644 --- a/apps/dav/tests/unit/ServerTest.php +++ b/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) { -- 2.39.5