diff --git a/apps/files_sharing/lib/Updater.php b/apps/files_sharing/lib/Updater.php index e2c0eef89706f..faa752a546d82 100644 --- a/apps/files_sharing/lib/Updater.php +++ b/apps/files_sharing/lib/Updater.php @@ -11,6 +11,7 @@ use OC\Files\Cache\FileAccess; use OC\Files\Filesystem; use OC\Files\Mount\MountPoint; +use OC\Share20\Manager; use OCP\Constants; use OCP\Files\Folder; use OCP\Files\Mount\IMountManager; @@ -64,6 +65,10 @@ private static function moveShareInOrOutOfShare($path): void { $shareManager = Server::get(\OCP\Share\IManager::class); + if ($shareManager instanceof Manager) { + $shareManager->restrictDownloadOfReshares($src); + } + // We intentionally include invalid shares, as they have been automatically invalidated due to the node no longer // being accessible for the user. Only in this case where we adjust the share after it was moved we want to ignore // this to be able to still adjust it. diff --git a/apps/files_sharing/tests/ReshareAttributesTest.php b/apps/files_sharing/tests/ReshareAttributesTest.php new file mode 100644 index 0000000000000..5f823d5fa140f --- /dev/null +++ b/apps/files_sharing/tests/ReshareAttributesTest.php @@ -0,0 +1,239 @@ +view->mkdir('folder'); + $this->view->mkdir('folder/sub'); + $this->view->file_put_contents('folder/sub/file.txt', $this->data); + } + + private function createShare(int $type, string $path, string $initiator, ?string $recipient = null): IShare { + $this->loginHelper($initiator); + if ($recipient !== null) { + return $this->share($type, $path, $initiator, $recipient, Constants::PERMISSION_ALL); + } + + $share = $this->shareManager->newShare() + ->setShareType($type) + ->setSharedBy($initiator) + ->setNode($this->rootFolder->getUserFolder($initiator)->get($path)) + ->setPermissions(Constants::PERMISSION_READ); + return $this->shareManager->createShare($share); + } + + private function setDownload(IShare $share, bool $canDownload): void { + $this->loginHelper($share->getSharedBy()); + $share = $this->shareManager->getShareById($share->getFullId()); + $attributes = $share->getAttributes() ?? $share->newAttributes(); + $attributes->setAttribute('permissions', 'download', $canDownload); + $share->setAttributes($attributes); + $this->shareManager->updateShare($share); + } + + private function canDownload(IShare $share): bool { + return $this->shareManager->getShareById($share->getFullId())->canDownload(); + } + + /** + * Moving between storages on object store primary storage creates new + * file ids, so the shares of the moved node no longer resolve. + */ + private function assertCannotServeDownloads(IShare $share): void { + $share = $this->shareManager->getShareById($share->getFullId()); + try { + $share->getNode(); + $nodeExists = true; + } catch (NotFoundException) { + $nodeExists = false; + } + $this->assertFalse($nodeExists && $share->canDownload()); + } + + public function testRestrictingDownloadRestrictsReshareChain(): void { + $share = $this->createShare(IShare::TYPE_USER, 'folder', self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2); + $reshare = $this->createShare(IShare::TYPE_USER, 'folder', self::TEST_FILES_SHARING_API_USER2, self::TEST_FILES_SHARING_API_USER3); + $link = $this->createShare(IShare::TYPE_LINK, 'folder', self::TEST_FILES_SHARING_API_USER2); + $fileLink = $this->createShare(IShare::TYPE_LINK, 'folder/sub/file.txt', self::TEST_FILES_SHARING_API_USER2); + $chainedLink = $this->createShare(IShare::TYPE_LINK, 'folder/sub', self::TEST_FILES_SHARING_API_USER3); + + $this->setDownload($share, false); + + $this->assertFalse($this->canDownload($reshare)); + $this->assertFalse($this->canDownload($link)); + $this->assertFalse($this->canDownload($fileLink)); + $this->assertFalse($this->canDownload($chainedLink)); + } + + public function testRestrictingDownloadKeepsSharesOfTheOwner(): void { + $share = $this->createShare(IShare::TYPE_USER, 'folder', self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2); + $ownerShare = $this->createShare(IShare::TYPE_USER, 'folder', self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER3); + $ownerLink = $this->createShare(IShare::TYPE_LINK, 'folder', self::TEST_FILES_SHARING_API_USER1); + + $this->setDownload($share, false); + + $this->assertTrue($this->canDownload($ownerShare)); + $this->assertTrue($this->canDownload($ownerLink)); + } + + public function testRestrictingDownloadKeepsResharesWithAnotherAllowingShare(): void { + $share = $this->createShare(IShare::TYPE_USER, 'folder', self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2); + $groupShare = $this->createShare(IShare::TYPE_GROUP, 'folder', self::TEST_FILES_SHARING_API_USER1, 'group1'); + $link = $this->createShare(IShare::TYPE_LINK, 'folder', self::TEST_FILES_SHARING_API_USER2); + + $this->setDownload($share, false); + $this->assertTrue($this->canDownload($link)); + + $this->setDownload($groupShare, false); + $this->assertFalse($this->canDownload($link)); + } + + public function testRestrictingDownloadRestrictsCyclicReshares(): void { + $share = $this->createShare(IShare::TYPE_USER, 'folder', self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2); + $this->createShare(IShare::TYPE_USER, 'folder', self::TEST_FILES_SHARING_API_USER2, self::TEST_FILES_SHARING_API_USER3); + $cyclicShare = $this->createShare(IShare::TYPE_GROUP, 'folder', self::TEST_FILES_SHARING_API_USER3, 'group1'); + $link = $this->createShare(IShare::TYPE_LINK, 'folder', self::TEST_FILES_SHARING_API_USER2); + + $this->setDownload($share, false); + + $this->assertFalse($this->canDownload($cyclicShare)); + $this->assertFalse($this->canDownload($link)); + } + + public function testRestrictingDownloadKeepsCyclicResharesReachableFromTheOwner(): void { + $share = $this->createShare(IShare::TYPE_USER, 'folder', self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2); + $otherShare = $this->createShare(IShare::TYPE_USER, 'folder', self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER3); + $this->createShare(IShare::TYPE_USER, 'folder', self::TEST_FILES_SHARING_API_USER2, self::TEST_FILES_SHARING_API_USER4); + $this->createShare(IShare::TYPE_GROUP, 'folder', self::TEST_FILES_SHARING_API_USER3, 'group1'); + $link = $this->createShare(IShare::TYPE_LINK, 'folder', self::TEST_FILES_SHARING_API_USER2); + + $this->setDownload($share, false); + $this->assertTrue($this->canDownload($link)); + + $this->setDownload($otherShare, false); + $this->assertFalse($this->canDownload($link)); + } + + public function testRestrictingDownloadOfReshareRestrictsItsReshares(): void { + $this->createShare(IShare::TYPE_USER, 'folder', self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2); + $reshare = $this->createShare(IShare::TYPE_USER, 'folder', self::TEST_FILES_SHARING_API_USER2, self::TEST_FILES_SHARING_API_USER3); + $link = $this->createShare(IShare::TYPE_LINK, 'folder', self::TEST_FILES_SHARING_API_USER2); + $chainedLink = $this->createShare(IShare::TYPE_LINK, 'folder', self::TEST_FILES_SHARING_API_USER3); + + $this->setDownload($reshare, false); + + $this->assertTrue($this->canDownload($link)); + $this->assertFalse($this->canDownload($chainedLink)); + } + + public function testAllowingDownloadAgainKeepsResharesRestricted(): void { + $share = $this->createShare(IShare::TYPE_USER, 'folder', self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2); + $link = $this->createShare(IShare::TYPE_LINK, 'folder', self::TEST_FILES_SHARING_API_USER2); + + $this->setDownload($share, false); + $this->setDownload($share, true); + + $this->assertFalse($this->canDownload($link)); + } + + public function testUpdatingPermissionsKeepsResharesUnchanged(): void { + $share = $this->createShare(IShare::TYPE_USER, 'folder', self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2); + $link = $this->createShare(IShare::TYPE_LINK, 'folder', self::TEST_FILES_SHARING_API_USER2); + + $this->loginHelper(self::TEST_FILES_SHARING_API_USER1); + $share = $this->shareManager->getShareById($share->getFullId()); + $share->setPermissions(Constants::PERMISSION_READ | Constants::PERMISSION_SHARE); + $this->shareManager->updateShare($share); + + $this->assertNull($this->shareManager->getShareById($link->getFullId())->getAttributes()); + } + + public function testRestrictingDownloadRestrictsResharesOfUnresolvableSharers(): void { + $share = $this->createShare(IShare::TYPE_USER, 'folder', self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2); + $link = $this->createShare(IShare::TYPE_LINK, 'folder', self::TEST_FILES_SHARING_API_USER2); + + $qb = Server::get(IDBConnection::class)->getQueryBuilder(); + $qb->update('share') + ->set('uid_initiator', $qb->createNamedParameter('non-existing-user')) + ->where($qb->expr()->eq('id', $qb->createNamedParameter($link->getId()))) + ->executeStatement(); + + $this->setDownload($share, false); + + $this->assertFalse($this->canDownload($link)); + } + + public function testMovingIntoRestrictedShareRestrictsReshares(): void { + $share = $this->createShare(IShare::TYPE_USER, 'folder', self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2); + $this->setDownload($share, false); + + $this->loginHelper(self::TEST_FILES_SHARING_API_USER2); + $view = new View('/' . self::TEST_FILES_SHARING_API_USER2 . '/files'); + $view->mkdir('own'); + $view->file_put_contents('own/file.txt', $this->data); + $link = $this->createShare(IShare::TYPE_LINK, 'own', self::TEST_FILES_SHARING_API_USER2); + $reshare = $this->createShare(IShare::TYPE_USER, 'own', self::TEST_FILES_SHARING_API_USER2, self::TEST_FILES_SHARING_API_USER3); + $chainedLink = $this->createShare(IShare::TYPE_LINK, 'own', self::TEST_FILES_SHARING_API_USER3); + + $this->loginHelper(self::TEST_FILES_SHARING_API_USER2); + $view->rename('own', 'folder/own'); + + $this->assertCannotServeDownloads($link); + $this->assertCannotServeDownloads($reshare); + $this->assertCannotServeDownloads($chainedLink); + } + + public function testMovingByOwnerIntoRestrictedShareRestrictsReshares(): void { + $this->view->mkdir('open'); + $this->view->mkdir('open/moved'); + $share = $this->createShare(IShare::TYPE_USER, 'folder', self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2); + $this->createShare(IShare::TYPE_USER, 'open', self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2); + $this->setDownload($share, false); + $link = $this->createShare(IShare::TYPE_LINK, 'open/moved', self::TEST_FILES_SHARING_API_USER2); + $this->assertTrue($this->canDownload($link)); + + $this->loginHelper(self::TEST_FILES_SHARING_API_USER1); + $this->view->rename('open/moved', 'folder/moved'); + + $this->assertFalse($this->canDownload($link)); + } + + public function testMovingIntoAllowingShareKeepsReshares(): void { + $this->createShare(IShare::TYPE_USER, 'folder', self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2); + + $this->loginHelper(self::TEST_FILES_SHARING_API_USER2); + $view = new View('/' . self::TEST_FILES_SHARING_API_USER2 . '/files'); + $view->mkdir('own'); + $link = $this->createShare(IShare::TYPE_LINK, 'own', self::TEST_FILES_SHARING_API_USER2); + + $this->loginHelper(self::TEST_FILES_SHARING_API_USER2); + $view->rename('own', 'folder/own'); + + $this->assertTrue($this->canDownload($link)); + } +} diff --git a/lib/private/Files/View.php b/lib/private/Files/View.php index 744a216d9b254..8839b78a36acd 100644 --- a/lib/private/Files/View.php +++ b/lib/private/Files/View.php @@ -1902,32 +1902,29 @@ private function targetIsNotShared(string $user, string $targetPath): bool { $providers = [ IShare::TYPE_USER, IShare::TYPE_GROUP, + IShare::TYPE_LINK, IShare::TYPE_EMAIL, IShare::TYPE_CIRCLE, IShare::TYPE_ROOM, IShare::TYPE_DECK, ]; $shareManager = Server::get(IManager::class); - /** @var IShare[] $shares */ - $shares = array_merge(...array_map(function (int $type) use ($shareManager, $user) { - return $shareManager->getSharesBy($user, $type); - }, $providers)); + $rootFolder = Server::get(IRootFolder::class); + $userFolderPath = '/' . $user . '/files'; - foreach ($shares as $share) { + for ($path = $targetPath; str_starts_with($path, $userFolderPath . '/'); $path = dirname($path)) { try { - $sharedPath = $share->getNode()->getPath(); - } catch (NotFoundException $e) { - // node is not found, ignoring - $this->logger->debug( - 'Could not find the node linked to a share', - ['app' => 'files', 'exception' => $e]); + $node = $rootFolder->get($path); + } catch (NotFoundException) { continue; } - if ($targetPath === $sharedPath || str_starts_with($targetPath, $sharedPath . '/')) { - $this->logger->debug( - 'It is not allowed to move one mount point into a shared folder', - ['app' => 'files']); - return false; + foreach ($providers as $type) { + if ($shareManager->getSharesBy($user, $type, $node, false, 1) !== []) { + $this->logger->debug( + 'It is not allowed to move one mount point into a shared folder', + ['app' => 'files']); + return false; + } } } diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index e89b3c50f10da..c93b0854d78ef 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -18,6 +18,7 @@ use OC\Share20\Exception\ProviderException; use OCA\Circles\Api\v1\Circles; use OCA\Files_Sharing\AppInfo\Application; +use OCA\Files_Sharing\SharedMount; use OCA\Files_Sharing\SharedStorage; use OCA\ShareByMail\ShareByMailProvider; use OCP\Constants; @@ -786,6 +787,12 @@ public function updateShare(IShare $share, bool $onlyValid = true): IShare { $share = $provider->update($share); } + if (in_array($share->getShareType(), [IShare::TYPE_USER, IShare::TYPE_GROUP, IShare::TYPE_CIRCLE, IShare::TYPE_ROOM, IShare::TYPE_DECK], true) + && $originalShare->getAttributes()?->getAttribute('permissions', 'download') !== false + && $share->getAttributes()?->getAttribute('permissions', 'download') === false) { + $this->restrictDownloadOfReshares($share->getNode(), [$share->getFullId() => true]); + } + if ($expirationDateUpdated === true) { \OC_Hook::emit(Share::class, 'post_set_expiration_date', [ 'itemType' => $share->getNode() instanceof File ? 'file' : 'folder', @@ -826,6 +833,120 @@ public function updateShare(IShare $share, bool $onlyValid = true): IShare { return $share; } + /** + * Restrict downloads of the reshares of the given node and its descendants + * whose sharer cannot download them through a chain of received shares + * that allow downloads, starting at the owner. + * + * @param array $restrictedShareIds full ids of received shares to treat as restricted + */ + public function restrictDownloadOfReshares(Node $node, array $restrictedShareIds = []): void { + $reshares = $this->getResharesOf($node); + + // Grow the allowed reshares from received shares outside of them, so that + // reshares which only allow each other (cycles) end up restricted. + $allowed = []; + $allowsDownload = function (IShare $receivedShare) use ($restrictedShareIds, $reshares, &$allowed): bool { + $id = $receivedShare->getFullId(); + return !isset($restrictedShareIds[$id]) + && $receivedShare->getAttributes()?->getAttribute('permissions', 'download') !== false + && (!isset($reshares[$id]) || isset($allowed[$id])); + }; + do { + $changed = false; + foreach ($reshares as $id => $reshare) { + if (isset($allowed[$id]) || $reshare->getAttributes()?->getAttribute('permissions', 'download') === false) { + continue; + } + if ($this->canDownloadThroughReceivedShares($reshare->getSharedBy(), $reshare->getNodeId(), $allowsDownload)) { + $allowed[$id] = true; + $changed = true; + } + } + } while ($changed); + + foreach ($reshares as $id => $reshare) { + if (isset($allowed[$id]) || $reshare->getAttributes()?->getAttribute('permissions', 'download') === false) { + continue; + } + $attributes = $reshare->getAttributes() ?? $reshare->newAttributes(); + $attributes->setAttribute('permissions', 'download', false); + $reshare->setAttributes($attributes); + $this->factory->getProviderForType($reshare->getShareType())->update($reshare); + } + } + + /** + * Get the shares of the node and its descendants that were not created by the node owner. + * + * @return array shares indexed by their full id + */ + private function getResharesOf(Node $node): array { + $internalPath = $node->getInternalPath(); + + $qb = $this->connection->getQueryBuilder(); + $qb->select('s.id', 's.share_type') + ->from('share', 's') + ->innerJoin('s', 'filecache', 'f', $qb->expr()->eq('s.file_source', 'f.fileid')) + ->where($qb->expr()->eq('f.storage', $qb->createNamedParameter($node->getStorage()->getCache()->getNumericStorageId(), IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->in('s.item_type', $qb->createNamedParameter(['file', 'folder'], IQueryBuilder::PARAM_STR_ARRAY))) + ->andWhere($qb->expr()->neq('s.share_type', $qb->createNamedParameter(IShare::TYPE_USERGROUP, IQueryBuilder::PARAM_INT))); + + $owner = $node->getOwner()?->getUID(); + if ($owner !== null) { + $qb->andWhere($qb->expr()->neq('s.uid_initiator', $qb->createNamedParameter($owner))); + } + if ($internalPath !== '') { + $qb->andWhere($qb->expr()->orX( + $qb->expr()->eq('f.fileid', $qb->createNamedParameter($node->getId(), IQueryBuilder::PARAM_INT)), + $qb->expr()->like('f.path', $qb->createNamedParameter($this->connection->escapeLikeParameter($internalPath) . '/%')), + )); + } + + $reshares = []; + $result = $qb->executeQuery(); + while ($row = $result->fetchAssociative()) { + try { + $reshare = $this->factory->getProviderForType((int)$row['share_type'])->getShareById((string)$row['id']); + $reshares[$reshare->getFullId()] = $reshare; + } catch (ProviderException|ShareNotFound) { + } + } + $result->closeCursor(); + + return $reshares; + } + + /** + * Whether the user can download the node through their own storage or + * through one of the shares they received it from. + * + * @param callable(IShare): bool $allowsDownload whether a received share allows downloads + */ + private function canDownloadThroughReceivedShares(string $userId, int $nodeId, callable $allowsDownload): bool { + try { + $nodes = $this->rootFolder->getUserFolder($userId)->getById($nodeId); + } catch (\Exception $e) { + $this->logger->warning('Could not resolve the received shares of ' . $userId . ', restricting downloads of their reshares', ['exception' => $e]); + return false; + } + + foreach ($nodes as $node) { + $mount = $node->getMountPoint(); + if (!$mount instanceof SharedMount) { + return true; + } + + foreach ($mount->getGroupedShares() as $receivedShare) { + if ($allowsDownload($receivedShare)) { + return true; + } + } + } + + return false; + } + #[Override] public function acceptShare(IShare $share, string $recipientId): IShare { [$providerId,] = $this->splitFullId($share->getFullId()); diff --git a/tests/lib/Files/ViewTest.php b/tests/lib/Files/ViewTest.php index 9ec20eea1f742..d0da9b5b2c4ba 100644 --- a/tests/lib/Files/ViewTest.php +++ b/tests/lib/Files/ViewTest.php @@ -30,7 +30,9 @@ use OCP\Files\ForbiddenException; use OCP\Files\GenericFileException; use OCP\Files\InvalidPathException; +use OCP\Files\IRootFolder; use OCP\Files\Mount\IMountManager; +use OCP\Files\Node; use OCP\Files\NotFoundException; use OCP\Files\Storage\IStorage; use OCP\Files\Storage\IStorageFactory; @@ -1790,6 +1792,46 @@ public function testMoveMountPointIntoSharedFolder(): void { $userObject->delete(); } + public function testMoveMountPointIntoLinkSharedFolder(): void { + self::loginAsUser(self::$user); + + [$mount1] = $this->createTestMovableMountPoints([ + self::$user . '/files/mount1', + ]); + $mount1->expects($this->never()) + ->method('moveMount'); + + $view = new View('/' . self::$user . '/files/'); + $userFolder = Server::get(IRootFolder::class)->getUserFolder(self::$user); + $shareManager = Server::get(IShareManager::class); + $shares = []; + // More links than a single page of shares, created before the shared target + for ($i = 0; $i <= 50; $i++) { + $view->mkdir('other' . $i); + $shares[] = $this->createLinkShare($shareManager, $userFolder->get('other' . $i)); + } + $view->mkdir('linkdir'); + $shares[] = $this->createLinkShare($shareManager, $userFolder->get('linkdir')); + + try { + $this->expectException(ForbiddenException::class); + $view->rename('mount1', 'linkdir/mount1'); + } finally { + foreach ($shares as $share) { + $shareManager->deleteShare($share); + } + } + } + + private function createLinkShare(IShareManager $shareManager, Node $node): IShare { + $share = $shareManager->newShare(); + $share->setSharedBy(self::$user) + ->setShareType(IShare::TYPE_LINK) + ->setPermissions(Constants::PERMISSION_READ) + ->setNode($node); + return $shareManager->createShare($share); + } + public static function basicOperationProviderForLocks(): array { return [ // --- write hook ----