diff --git a/.github/workflows/integration-sqlite.yml b/.github/workflows/integration-sqlite.yml index 0800f8be377a7..5b10efca08bc7 100644 --- a/.github/workflows/integration-sqlite.yml +++ b/.github/workflows/integration-sqlite.yml @@ -129,6 +129,7 @@ jobs: extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, imagick, intl, json, ldap, libxml, mbstring, openssl, pcntl, posix, redis, session, simplexml, xmlreader, xmlwriter, zip, zlib, sqlite, pdo_sqlite coverage: none ini-file: development + ini-values: disable_functions="" env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/apps/dav/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php index 6d088a554073d..8eb3f0069aba0 100644 --- a/apps/dav/lib/Connector/Sabre/ServerFactory.php +++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php @@ -185,7 +185,8 @@ public function createServer( $server->addPlugin(new SharesPlugin( $tree, $this->userSession, - \OCP\Server::get(\OCP\Share\IManager::class) + \OCP\Server::get(\OCP\Share\IManager::class), + \OCP\Server::get(IRootFolder::class), )); $server->addPlugin(new CommentPropertiesPlugin(\OCP\Server::get(ICommentsManager::class), $this->userSession)); $server->addPlugin(new FilesReportPlugin( diff --git a/apps/dav/lib/Connector/Sabre/SharesPlugin.php b/apps/dav/lib/Connector/Sabre/SharesPlugin.php index 5a4f187ef3c94..3b2e3ca3e8361 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; @@ -51,6 +52,7 @@ public function __construct( private Tree $tree, IUserSession $userSession, private IManager $shareManager, + private IRootFolder $rootFolder, ) { $this->userId = $userSession->getUser()->getUID(); } @@ -82,7 +84,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, @@ -104,6 +106,10 @@ private function getShare(Node $node): array { -1 ); + if (!$includeIncoming) { + continue; + } + // Also check for shares where the user is the recipient try { $result[] = $this->shareManager->getSharedWith( @@ -120,6 +126,24 @@ private function getShare(Node $node): array { return array_merge(...$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[][] @@ -235,8 +259,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; } @@ -257,7 +281,7 @@ public function validateMoveOrCopy(string $source, string $target): bool { // the user moving the file out of the share to their home storage would give them share permissions and allow moving into the share // // since the 2-step move is allowed, we also allow both steps at once - if ($sourceNode->isDeletable()) { + if ($sourceNode->getInternalPath() !== '' && $sourceNode->isDeletable()) { return true; } } diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index f3d5dd00242ec..c957fa2be870e 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -340,6 +340,7 @@ public function __construct( $this->server->tree, $userSession, $shareManager, + \OCP\Server::get(IRootFolder::class), )); $this->server->addPlugin(new CommentPropertiesPlugin( \OCP\Server::get(ICommentsManager::class), diff --git a/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php index ff6d7f3c3dd42..ba895383c057d 100644 --- a/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php @@ -14,6 +14,7 @@ use OCA\DAV\Connector\Sabre\SharesPlugin; use OCA\DAV\Upload\UploadFile; use OCP\Files\Folder; +use OCP\Files\IRootFolder; use OCP\IUser; use OCP\IUserSession; use OCP\Share\IManager; @@ -27,6 +28,7 @@ class SharesPluginTest extends \Test\TestCase { private \Sabre\DAV\Server $server; private \Sabre\DAV\Tree&MockObject $tree; private \OCP\Share\IManager&MockObject $shareManager; + private IRootFolder&MockObject $rootFolder; private SharesPlugin $plugin; protected function setUp(): void { @@ -34,6 +36,7 @@ protected function setUp(): void { $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') @@ -46,7 +49,8 @@ protected function setUp(): void { $this->plugin = new SharesPlugin( $this->tree, $userSession, - $this->shareManager + $this->shareManager, + $this->rootFolder, ); $this->plugin->initialize($this->server); } diff --git a/build/integration/sharing_features/sharing-v1-part4.feature b/build/integration/sharing_features/sharing-v1-part4.feature index 5e710d7f28647..4409ebb1116be 100644 --- a/build/integration/sharing_features/sharing-v1-part4.feature +++ b/build/integration/sharing_features/sharing-v1-part4.feature @@ -316,6 +316,117 @@ Scenario: Can copy file between shares if share permissions 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" + Scenario: Group deletes removes mount without marking Given As an "admin" And user "user0" exists