diff --git a/apps/dav/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php index ce753f8f32128..9ac004c264f71 100644 --- a/apps/dav/lib/Connector/Sabre/ServerFactory.php +++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php @@ -16,6 +16,7 @@ use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Folder; use OCP\Files\IFilenameValidator; +use OCP\Files\IRootFolder; use OCP\Files\Mount\IMountManager; use OCP\IConfig; use OCP\IDBConnection; @@ -152,8 +153,8 @@ public function createServer(string $baseUri, $server->addPlugin(new \OCA\DAV\Connector\Sabre\SharesPlugin( $objectTree, $this->userSession, - $userFolder, - \OC::$server->getShareManager() + \OCP\Server::get(\OCP\Share\IManager::class), + \OCP\Server::get(IRootFolder::class), )); $server->addPlugin(new \OCA\DAV\Connector\Sabre\CommentPropertiesPlugin(\OC::$server->getCommentsManager(), $this->userSession)); $server->addPlugin(new \OCA\DAV\Connector\Sabre\FilesReportPlugin( diff --git a/apps/dav/lib/Connector/Sabre/SharesPlugin.php b/apps/dav/lib/Connector/Sabre/SharesPlugin.php index 258ef01427027..dbe2c3c227afc 100644 --- a/apps/dav/lib/Connector/Sabre/SharesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/SharesPlugin.php @@ -11,6 +11,7 @@ use OCA\DAV\Connector\Sabre\Exception\Forbidden; use OCA\DAV\Connector\Sabre\Node as DavNode; use OCP\Files\Folder; +use OCP\Files\IRootFolder; use OCP\Files\Node; use OCP\Files\NotFoundException; use OCP\Files\Storage\ISharedStorage; @@ -47,8 +48,8 @@ class SharesPlugin extends \Sabre\DAV\ServerPlugin { public function __construct( private Tree $tree, private IUserSession $userSession, - private Folder $userFolder, private IManager $shareManager, + private IRootFolder $rootFolder, ) { $this->userId = $userSession->getUser()->getUID(); } @@ -79,7 +80,7 @@ public function initialize(Server $server) { * @param Node $node * @return IShare[] */ - private function getShare(Node $node): array { + private function getShare(Node $node, bool $includeIncoming = true): array { $result = []; $requestedShareTypes = [ IShare::TYPE_USER, @@ -102,6 +103,10 @@ private function getShare(Node $node): array { -1 )); + if (!$includeIncoming) { + continue; + } + // Also check for shares where the user is the recipient try { $result = array_merge($result, $this->shareManager->getSharedWith( @@ -118,6 +123,24 @@ private function getShare(Node $node): array { return $result; } + /** + * @return IShare[] + */ + private function getSharesForTarget(Node $node): array { + $shares = $this->getShare($node); + if ($shares !== []) { + return $shares; + } + + // also check the owner side + $userRoot = $this->rootFolder->getUserFolder($this->userId); + while (str_starts_with($node->getPath(), $userRoot->getPath() . '/')) { + $shares = array_merge($shares, $this->getShare($node, false)); + $node = $node->getParent(); + } + return $shares; + } + /** * @param Folder $node * @return IShare[][] @@ -233,8 +256,8 @@ public function validateMoveOrCopy(string $source, string $target): bool { return true; } - $targetShares = $this->getShare($targetNode->getNode()); - if (empty($targetShares)) { + $targetShares = $this->getSharesForTarget($targetNode->getNode()); + if ($targetShares === []) { // Target is not a share so no re-sharing inprogress return true; } diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index 586f155078a29..be453ca50eacb 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -58,6 +58,7 @@ use OCP\Diagnostics\IEventLogger; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\IFilenameValidator; +use OCP\Files\IRootFolder; use OCP\FilesMetadata\IFilesMetadataManager; use OCP\IAppConfig; use OCP\ICacheFactory; @@ -290,8 +291,8 @@ public function __construct(IRequest $request, string $baseUri) { $this->server->addPlugin(new SharesPlugin( $this->server->tree, $userSession, - $userFolder, $shareManager, + \OCP\Server::get(IRootFolder::class), )); $this->server->addPlugin(new CommentPropertiesPlugin( \OC::$server->getCommentsManager(), diff --git a/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php index 546be840cd8f9..d80f824333d04 100644 --- a/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php @@ -12,10 +12,12 @@ use OCA\DAV\Connector\Sabre\Node; use OCA\DAV\Upload\UploadFile; use OCP\Files\Folder; +use OCP\Files\IRootFolder; use OCP\IUser; use OCP\IUserSession; use OCP\Share\IManager; use OCP\Share\IShare; +use PHPUnit\Framework\MockObject\MockObject; use Sabre\DAV\Tree; class SharesPluginTest extends \Test\TestCase { @@ -37,20 +39,21 @@ class SharesPluginTest extends \Test\TestCase { private $shareManager; /** - * @var \OCP\Files\Folder + * @var \OCA\DAV\Connector\Sabre\SharesPlugin */ - private $userFolder; + private $plugin; /** - * @var \OCA\DAV\Connector\Sabre\SharesPlugin + * @var IRootFolder&MockObject */ - private $plugin; + private $rootFolder; protected function setUp(): void { parent::setUp(); $this->server = new \Sabre\DAV\Server(); $this->tree = $this->createMock(Tree::class); $this->shareManager = $this->createMock(IManager::class); + $this->rootFolder = $this->createMock(IRootFolder::class); $user = $this->createMock(IUser::class); $user->expects($this->once()) ->method('getUID') @@ -59,13 +62,12 @@ protected function setUp(): void { $userSession->expects($this->once()) ->method('getUser') ->willReturn($user); - $this->userFolder = $this->createMock(Folder::class); $this->plugin = new \OCA\DAV\Connector\Sabre\SharesPlugin( $this->tree, $userSession, - $this->userFolder, - $this->shareManager + $this->shareManager, + $this->rootFolder, ); $this->plugin->initialize($this->server); } diff --git a/build/integration/dav_features/dav-v2-public.feature b/build/integration/dav_features/dav-v2-public.feature index 82e45c6d6b1a6..3bb94a021719d 100644 --- a/build/integration/dav_features/dav-v2-public.feature +++ b/build/integration/dav_features/dav-v2-public.feature @@ -57,34 +57,6 @@ Feature: dav-v2-public When Downloading public file "/image.png" without ajax header Then the downloaded file has the content of "/testshare/image.png" from "user1" data - Scenario: Finalizing a public chunked upload with COPY is not allowed - Given using new dav path - And user "user0" exists - And As an "user0" - And user "user0" created a folder "/public-upload" - And as "user0" creating a share with - | path | public-upload | - | shareType | 3 | - | publicUpload | true | - And creating a new public chunking upload with id "chunking-public-copy" - And uploading new public chunk file "1" with "AAAAA" to id "chunking-public-copy" - When copying new public chunk file with id "chunking-public-copy" to "/target.txt" - Then the HTTP status code should be "409" - # Then the HTTP status code should be "405" - - Scenario: Finalizing a public chunked upload with MOVE overwrites the target - Given using new dav path - And user "user0" exists - And As an "user0" - And user "user0" created a folder "/public-upload" - And User "user0" uploads file with content "original content" to "/public-upload/target.txt" - And as "user0" creating a share with - | path | public-upload | - | shareType | 3 | - | publicUpload | true | - And creating a new public chunking upload with id "chunking-public-move" - And uploading new public chunk file "1" with "AAAAA" to id "chunking-public-move" - When moving new public chunk file with id "chunking-public-move" to "/target.txt" - Then the HTTP status code should be "204" - And Downloading file "/public-upload/target.txt" as "user0" - Then Downloaded content should be "AAAAA" + # The chunked upload scenarios of this feature are not backported: + # `/public.php/dav/uploads/` does not exist on this branch, the public + # DAV endpoint only serves `/public.php/dav/files/`. diff --git a/build/integration/sharing_features/sharing-v1-part4.feature b/build/integration/sharing_features/sharing-v1-part4.feature index fbb2803d6904d..06ef043709bf8 100644 --- a/build/integration/sharing_features/sharing-v1-part4.feature +++ b/build/integration/sharing_features/sharing-v1-part4.feature @@ -289,3 +289,114 @@ Scenario: Can copy file between shares if share permissions And the OCS status code should be "100" When User "user1" copies file "/share/test.txt" to "/re-share/movetest.txt" Then the HTTP status code should be "201" + +Scenario: Cannot copy files from share without share permission into subfolder of other share + Given user "user0" exists + Given user "user1" exists + Given user "user2" exists + And As an "user0" + And user "user0" created a folder "/share" + When creating a share with + | path | share | + | shareType | 0 | + | shareWith | user1 | + | permissions | 7 | + Then the HTTP status code should be "200" + And the OCS status code should be "100" + And User "user0" uploads file with content "test" to "/share/test.txt" + And As an "user1" + And user "user1" created a folder "/re-share" + And user "user1" created a folder "/re-share/subfolder" + When creating a share with + | path | re-share | + | shareType | 0 | + | shareWith | user2 | + | permissions | 31 | + Then the HTTP status code should be "200" + And the OCS status code should be "100" + When User "user1" copies file "/share/test.txt" to "/re-share/subfolder/copytest.txt" + Then the HTTP status code should be "403" + +Scenario: Cannot move files from share without share permission into subfolder of other share + Given user "user0" exists + Given user "user1" exists + Given user "user2" exists + And As an "user0" + And user "user0" created a folder "/share" + When creating a share with + | path | share | + | shareType | 0 | + | shareWith | user1 | + | permissions | 7 | + Then the HTTP status code should be "200" + And the OCS status code should be "100" + And User "user0" uploads file with content "test" to "/share/test.txt" + And As an "user1" + And user "user1" created a folder "/re-share" + And user "user1" created a folder "/re-share/subfolder" + When creating a share with + | path | re-share | + | shareType | 0 | + | shareWith | user2 | + | permissions | 31 | + Then the HTTP status code should be "200" + And the OCS status code should be "100" + When User "user1" moves file "/share/test.txt" to "/re-share/subfolder/movetest.txt" + Then the HTTP status code should be "403" + +Scenario: Cannot move folder containing share without share permission into subfolder of other share + Given user "user0" exists + Given user "user1" exists + Given user "user2" exists + And As an "user0" + And user "user0" created a folder "/share" + When creating a share with + | path | share | + | shareType | 0 | + | shareWith | user1 | + | permissions | 7 | + Then the HTTP status code should be "200" + And the OCS status code should be "100" + And User "user0" uploads file with content "test" to "/share/test.txt" + And As an "user1" + And user "user1" created a folder "/contains-share" + When User "user1" moves file "/share" to "/contains-share/share" + Then the HTTP status code should be "201" + And user "user1" created a folder "/re-share" + And user "user1" created a folder "/re-share/subfolder" + When creating a share with + | path | re-share | + | shareType | 0 | + | shareWith | user2 | + | permissions | 31 | + Then the HTTP status code should be "200" + And the OCS status code should be "100" + When User "user1" moves file "/contains-share" to "/re-share/subfolder/movetest" + Then the HTTP status code should be "403" + +Scenario: Can copy file between shares into subfolder if share permissions + Given user "user0" exists + Given user "user1" exists + Given user "user2" exists + And As an "user0" + And user "user0" created a folder "/share" + When creating a share with + | path | share | + | shareType | 0 | + | shareWith | user1 | + | permissions | 31 | + Then the HTTP status code should be "200" + And the OCS status code should be "100" + And User "user0" uploads file with content "test" to "/share/test.txt" + And As an "user1" + And user "user1" created a folder "/re-share" + And user "user1" created a folder "/re-share/subfolder" + When creating a share with + | path | re-share | + | shareType | 0 | + | shareWith | user2 | + | permissions | 31 | + Then the HTTP status code should be "200" + And the OCS status code should be "100" + When User "user1" copies file "/share/test.txt" to "/re-share/subfolder/copytest.txt" + Then the HTTP status code should be "201"