From 5606cadae7cae53c0a5397818a78854233e80bc0 Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Tue, 13 Feb 2024 14:46:04 +0100 Subject: [PATCH 1/5] 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 4c57f3412e3f9..755d13f837184 100644 --- a/apps/dav/lib/Connector/Sabre/ServerFactory.php +++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php @@ -161,7 +161,7 @@ public function createServer(string $baseUri, // 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 51e3622142db3..0ae472460beda 100644 --- a/apps/dav/lib/DAV/ViewOnlyPlugin.php +++ b/apps/dav/lib/DAV/ViewOnlyPlugin.php @@ -24,8 +24,8 @@ 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\Server; use Sabre\DAV\ServerPlugin; use Sabre\HTTP\RequestInterface; @@ -36,10 +36,12 @@ */ 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 @@ public function checkViewOnly(RequestInterface $request): bool { $node = $davNode->getNode(); } else if ($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 37b5eb3b70b1a..809b804796db2 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -236,7 +236,7 @@ public function __construct(IRequest $request, string $baseUri) { // 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 e0f6f1302a5ab..a00a04d147f17 100644 --- a/apps/dav/tests/unit/DAV/ViewOnlyPluginTest.php +++ b/apps/dav/tests/unit/DAV/ViewOnlyPluginTest.php @@ -26,10 +26,11 @@ 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 @@ public function testCanGet(bool $isVersion, ?bool $attrEnabled, bool $expectCanD $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 62e2accd697d3..26309d5fcd487 100644 --- a/apps/dav/tests/unit/ServerTest.php +++ b/apps/dav/tests/unit/ServerTest.php @@ -45,6 +45,7 @@ public function test($uri, array $plugins): void { /** @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) { From b48b153ac683789bdeecbb3ad66cef9cf37fef38 Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Tue, 13 Feb 2024 12:39:31 +0100 Subject: [PATCH 2/5] Check node permissions when deleting a version Signed-off-by: Louis Chemineau --- .../lib/Versions/LegacyVersionsBackend.php | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/apps/files_versions/lib/Versions/LegacyVersionsBackend.php b/apps/files_versions/lib/Versions/LegacyVersionsBackend.php index 0820266d627a8..3ae6d31a428e5 100644 --- a/apps/files_versions/lib/Versions/LegacyVersionsBackend.php +++ b/apps/files_versions/lib/Versions/LegacyVersionsBackend.php @@ -27,6 +27,7 @@ namespace OCA\Files_Versions\Versions; use OC\Files\View; +use OCA\DAV\Connector\Sabre\Exception\Forbidden; use OCA\Files_Sharing\ISharedStorage; use OCA\Files_Sharing\SharedStorage; use OCA\Files_Versions\Db\VersionEntity; @@ -42,23 +43,27 @@ use OCP\Files\Storage\IStorage; use OCP\IUser; use OCP\IUserManager; +use OCP\IUserSession; class LegacyVersionsBackend implements IVersionBackend, INameableVersionBackend, IDeletableVersionBackend, INeedSyncVersionBackend { private IRootFolder $rootFolder; private IUserManager $userManager; private VersionsMapper $versionsMapper; private IMimeTypeLoader $mimeTypeLoader; + private IUserSession $userSession; public function __construct( IRootFolder $rootFolder, IUserManager $userManager, VersionsMapper $versionsMapper, - IMimeTypeLoader $mimeTypeLoader + IMimeTypeLoader $mimeTypeLoader, + IUserSession $userSession, ) { $this->rootFolder = $rootFolder; $this->userManager = $userManager; $this->versionsMapper = $versionsMapper; $this->mimeTypeLoader = $mimeTypeLoader; + $this->userSession = $userSession; } public function useBackendForStorage(IStorage $storage): bool { @@ -232,6 +237,10 @@ public function setVersionLabel(IVersion $version, string $label): void { } public function deleteVersion(IVersion $version): void { + if (!$this->currentUserHasPermissions($version, \OCP\Constants::PERMISSION_DELETE)) { + throw new Forbidden('You cannot delete this version because you do not have delete permissions on the source file.'); + } + Storage::deleteRevision($version->getVersionPath(), $version->getRevisionId()); $versionEntity = $this->versionsMapper->findVersionForFileId( $version->getSourceFile()->getId(), @@ -271,4 +280,23 @@ public function updateVersionEntity(File $sourceFile, int $revision, array $prop public function deleteVersionsEntity(File $file): void { $this->versionsMapper->deleteAllVersionsForFileId($file->getId()); } + + private function currentUserHasPermissions(IVersion $version, int $permissions): bool { + $sourceFile = $version->getSourceFile(); + $currentUserId = $this->userSession->getUser()?->getUID(); + + if ($currentUserId === null) { + throw new NotFoundException("No user logged in"); + } + + if ($sourceFile->getOwner()?->getUID() !== $currentUserId) { + $nodes = $this->rootFolder->getUserFolder($currentUserId)->getById($sourceFile->getId()); + $sourceFile = array_pop($nodes); + if (!$sourceFile) { + throw new NotFoundException("Version file not accessible by current user"); + } + } + + return ($sourceFile->getPermissions() & $permissions) === $permissions; + } } From d8026de6b38aeacdc8a409339b7cc77f99835789 Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Tue, 13 Feb 2024 12:51:01 +0100 Subject: [PATCH 3/5] Check node permissions when restoring a version Signed-off-by: Louis Chemineau --- apps/files_versions/lib/Versions/LegacyVersionsBackend.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/files_versions/lib/Versions/LegacyVersionsBackend.php b/apps/files_versions/lib/Versions/LegacyVersionsBackend.php index 3ae6d31a428e5..85ad0cd671da3 100644 --- a/apps/files_versions/lib/Versions/LegacyVersionsBackend.php +++ b/apps/files_versions/lib/Versions/LegacyVersionsBackend.php @@ -179,6 +179,10 @@ public function createVersion(IUser $user, FileInfo $file) { } public function rollback(IVersion $version) { + if (!$this->currentUserHasPermissions($version, \OCP\Constants::PERMISSION_UPDATE)) { + throw new Forbidden('You cannot restore this version because you do not have update permissions on the source file.'); + } + return Storage::rollback($version->getVersionPath(), $version->getRevisionId(), $version->getUser()); } From 1c4cfa1ee088138dae19ba66aca84bd1d37326c5 Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Wed, 21 Feb 2024 14:34:12 +0100 Subject: [PATCH 4/5] Check permissions when labeling a version Signed-off-by: Louis Chemineau --- apps/files_versions/lib/Versions/LegacyVersionsBackend.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/files_versions/lib/Versions/LegacyVersionsBackend.php b/apps/files_versions/lib/Versions/LegacyVersionsBackend.php index 85ad0cd671da3..8b2d2caf40ef8 100644 --- a/apps/files_versions/lib/Versions/LegacyVersionsBackend.php +++ b/apps/files_versions/lib/Versions/LegacyVersionsBackend.php @@ -229,6 +229,10 @@ public function getVersionFile(IUser $user, FileInfo $sourceFile, $revision): Fi } public function setVersionLabel(IVersion $version, string $label): void { + if (!$this->currentUserHasPermissions($version, \OCP\Constants::PERMISSION_UPDATE)) { + throw new Forbidden('You cannot label this version because you do not have update permissions on the source file.'); + } + $versionEntity = $this->versionsMapper->findVersionForFileId( $version->getSourceFile()->getId(), $version->getTimestamp(), From 0820481e83e582e4015aa6602bf2bea331ef8c1d Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Wed, 21 Feb 2024 16:47:19 +0100 Subject: [PATCH 5/5] Hide some actions based on node permissions and share attributes Signed-off-by: Louis Chemineau Signed-off-by: nextcloud-command Signed-off-by: Louis Chemineau --- .../files_versions/src/components/Version.vue | 39 +++++++++++++++++-- dist/files_versions-files_versions.js | 4 +- ...les_versions-files_versions.js.LICENSE.txt | 22 +++++++++++ dist/files_versions-files_versions.js.map | 2 +- 4 files changed, 60 insertions(+), 7 deletions(-) diff --git a/apps/files_versions/src/components/Version.vue b/apps/files_versions/src/components/Version.vue index 36f429b26c1ce..a5f5c05b6d57d 100644 --- a/apps/files_versions/src/components/Version.vue +++ b/apps/files_versions/src/components/Version.vue @@ -47,7 +47,7 @@