diff --git a/apps/federatedfilesharing/lib/FederatedShareProvider.php b/apps/federatedfilesharing/lib/FederatedShareProvider.php index 52b10cd9315f3..d504a3e2acc2c 100644 --- a/apps/federatedfilesharing/lib/FederatedShareProvider.php +++ b/apps/federatedfilesharing/lib/FederatedShareProvider.php @@ -638,7 +638,7 @@ public function restore(IShare $share, string $recipient): IShare { } - public function getSharesInFolder($userId, Folder $node, $reshares) { + public function getSharesInFolder($userId, Folder $node, $reshares, $shallow = true) { $qb = $this->dbConnection->getQueryBuilder(); $qb->select('*') ->from('share', 's') @@ -664,8 +664,13 @@ public function getSharesInFolder($userId, Folder $node, $reshares) { ); } - $qb->innerJoin('s', 'filecache' ,'f', $qb->expr()->eq('s.file_source', 'f.fileid')); - $qb->andWhere($qb->expr()->eq('f.parent', $qb->createNamedParameter($node->getId()))); + $qb->innerJoin('s', 'filecache', 'f', $qb->expr()->eq('s.file_source', 'f.fileid')); + + if ($shallow) { + $qb->andWhere($qb->expr()->eq('f.parent', $qb->createNamedParameter($node->getId()))); + } else { + $qb->andWhere($qb->expr()->like('f.path', $qb->createNamedParameter($this->dbConnection->escapeLikeParameter($node->getInternalPath()) . '/%'))); + } $qb->orderBy('id'); diff --git a/apps/files_sharing/lib/Updater.php b/apps/files_sharing/lib/Updater.php index ad194dde01610..59e06e6029982 100644 --- a/apps/files_sharing/lib/Updater.php +++ b/apps/files_sharing/lib/Updater.php @@ -29,6 +29,7 @@ use OC\Files\Mount\MountPoint; use OCP\Constants; use OCP\Share\IShare; +use OCP\Files\Folder; class Updater { @@ -37,7 +38,7 @@ class Updater { */ public static function renameHook($params) { self::renameChildren($params['oldpath'], $params['newpath']); - self::moveShareToShare($params['newpath']); + self::moveShareInOrOutOfShare($params['newpath']); } /** @@ -50,7 +51,7 @@ public static function renameHook($params) { * * @param string $path */ - private static function moveShareToShare($path) { + private static function moveShareInOrOutOfShare($path): void { $userFolder = \OC::$server->getUserFolder(); // If the user folder can't be constructed (e.g. link share) just return. @@ -62,10 +63,18 @@ private static function moveShareToShare($path) { $shareManager = \OC::$server->getShareManager(); + // FIXME: should CIRCLES be included here ?? $shares = $shareManager->getSharesBy($userFolder->getOwner()->getUID(), IShare::TYPE_USER, $src, false, -1); $shares = array_merge($shares, $shareManager->getSharesBy($userFolder->getOwner()->getUID(), IShare::TYPE_GROUP, $src, false, -1)); $shares = array_merge($shares, $shareManager->getSharesBy($userFolder->getOwner()->getUID(), IShare::TYPE_ROOM, $src, false, -1)); + if ($src instanceof Folder) { + $subShares = $shareManager->getSharesInFolder($userFolder->getOwner()->getUID(), $src, false, false); + foreach ($subShares as $subShare) { + $shares = array_merge($shares, array_values($subShare)); + } + } + // If the path we move is not a share we don't care if (empty($shares)) { return; @@ -74,21 +83,31 @@ private static function moveShareToShare($path) { // Check if the destination is inside a share $mountManager = \OC::$server->getMountManager(); $dstMount = $mountManager->find($src->getPath()); - if (!($dstMount instanceof \OCA\Files_Sharing\SharedMount)) { - return; - } - - $newOwner = $dstMount->getShare()->getShareOwner(); //Ownership is moved over foreach ($shares as $share) { - /** @var IShare $share */ - if (!($dstMount->getShare()->getPermissions() & Constants::PERMISSION_SHARE)) { - $shareManager->deleteShare($share); + if ( + $share->getShareType() !== IShare::TYPE_USER && + $share->getShareType() !== IShare::TYPE_GROUP && + $share->getShareType() !== IShare::TYPE_ROOM + ) { continue; } + + if ($dstMount instanceof \OCA\Files_Sharing\SharedMount) { + if (!($dstMount->getShare()->getPermissions() & Constants::PERMISSION_SHARE)) { + $shareManager->deleteShare($share); + continue; + } + $newOwner = $dstMount->getShare()->getShareOwner(); + $newPermissions = $share->getPermissions() & $dstMount->getShare()->getPermissions(); + } else { + $newOwner = $userFolder->getOwner()->getUID(); + $newPermissions = $share->getPermissions(); + } + $share->setShareOwner($newOwner); - $share->setPermissions($share->getPermissions() & $dstMount->getShare()->getPermissions()); + $share->setPermissions($newPermissions); $shareManager->updateShare($share); } } diff --git a/apps/files_sharing/tests/UpdaterTest.php b/apps/files_sharing/tests/UpdaterTest.php index 464dff412a8f6..fbdfad2f9cbdc 100644 --- a/apps/files_sharing/tests/UpdaterTest.php +++ b/apps/files_sharing/tests/UpdaterTest.php @@ -237,4 +237,123 @@ public function testRename() { // cleanup $this->shareManager->deleteShare($share); } + + /** + * If a folder gets moved into shared folder, children shares should have their uid_owner and permissions adjusted + * user1 + * |-folder1 --> shared with user2 + * user2 + * |-folder2 --> shared with user3 and moved into folder1 + * |-subfolder1 --> shared with user3 + * |-file1.txt --> shared with user3 + * |-subfolder2 + * |-file2.txt --> shared with user3 + */ + public function testMovedIntoShareChangeOwner() { + $this->markTestSkipped('Skipped because this is failing with S3 as primary as file id are change when moved.'); + + // user1 creates folder1 + $viewUser1 = new \OC\Files\View('/' . self::TEST_FILES_SHARING_API_USER1 . '/files'); + $folder1 = 'folder1'; + $viewUser1->mkdir($folder1); + + // user1 shares folder1 to user2 + $folder1Share = $this->share( + IShare::TYPE_USER, + $folder1, + self::TEST_FILES_SHARING_API_USER1, + self::TEST_FILES_SHARING_API_USER2, + \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_SHARE + ); + + $this->loginHelper(self::TEST_FILES_SHARING_API_USER2); + $viewUser2 = new \OC\Files\View('/' . self::TEST_FILES_SHARING_API_USER2 . '/files'); + // Create user2 files + $folder2 = 'folder2'; + $viewUser2->mkdir($folder2); + $file1 = 'folder2/file1.txt'; + $viewUser2->touch($file1); + $subfolder1 = 'folder2/subfolder1'; + $viewUser2->mkdir($subfolder1); + $subfolder2 = 'folder2/subfolder2'; + $viewUser2->mkdir($subfolder2); + $file2 = 'folder2/subfolder2/file2.txt'; + $viewUser2->touch($file2); + + // user2 shares folder2 to user3 + $folder2Share = $this->share( + IShare::TYPE_USER, + $folder2, + self::TEST_FILES_SHARING_API_USER2, + self::TEST_FILES_SHARING_API_USER3, + \OCP\Constants::PERMISSION_ALL + ); + // user2 shares folder2/file1 to user3 + $file1Share = $this->share( + IShare::TYPE_USER, + $file1, + self::TEST_FILES_SHARING_API_USER2, + self::TEST_FILES_SHARING_API_USER3, + \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_SHARE + ); + // user2 shares subfolder1 to user3 + $subfolder1Share = $this->share( + IShare::TYPE_USER, + $subfolder1, + self::TEST_FILES_SHARING_API_USER2, + self::TEST_FILES_SHARING_API_USER3, + \OCP\Constants::PERMISSION_ALL + ); + // user2 shares subfolder2/file2.txt to user3 + $file2Share = $this->share( + IShare::TYPE_USER, + $file2, + self::TEST_FILES_SHARING_API_USER2, + self::TEST_FILES_SHARING_API_USER3, + \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_SHARE + ); + + // user2 moves folder2 into folder1 + $viewUser2->rename($folder2, $folder1.'/'.$folder2); + $folder2Share = $this->shareManager->getShareById($folder2Share->getFullId()); + $file1Share = $this->shareManager->getShareById($file1Share->getFullId()); + $subfolder1Share = $this->shareManager->getShareById($subfolder1Share->getFullId()); + $file2Share = $this->shareManager->getShareById($file2Share->getFullId()); + + // Expect uid_owner of both shares to be user1 + $this->assertEquals(self::TEST_FILES_SHARING_API_USER1, $folder2Share->getShareOwner()); + $this->assertEquals(self::TEST_FILES_SHARING_API_USER1, $file1Share->getShareOwner()); + $this->assertEquals(self::TEST_FILES_SHARING_API_USER1, $subfolder1Share->getShareOwner()); + $this->assertEquals(self::TEST_FILES_SHARING_API_USER1, $file2Share->getShareOwner()); + // Expect permissions to be limited by the permissions of the destination share + $this->assertEquals(\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_SHARE, $folder2Share->getPermissions()); + $this->assertEquals(\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_SHARE, $file1Share->getPermissions()); + $this->assertEquals(\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_SHARE, $subfolder1Share->getPermissions()); + $this->assertEquals(\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_SHARE, $file2Share->getPermissions()); + + // user2 moves folder2 out of folder1 + $viewUser2->rename($folder1.'/'.$folder2, $folder2); + $folder2Share = $this->shareManager->getShareById($folder2Share->getFullId()); + $file1Share = $this->shareManager->getShareById($file1Share->getFullId()); + $subfolder1Share = $this->shareManager->getShareById($subfolder1Share->getFullId()); + $file2Share = $this->shareManager->getShareById($file2Share->getFullId()); + + // Expect uid_owner of both shares to be user2 + $this->assertEquals(self::TEST_FILES_SHARING_API_USER2, $folder2Share->getShareOwner()); + $this->assertEquals(self::TEST_FILES_SHARING_API_USER2, $file1Share->getShareOwner()); + $this->assertEquals(self::TEST_FILES_SHARING_API_USER2, $subfolder1Share->getShareOwner()); + $this->assertEquals(self::TEST_FILES_SHARING_API_USER2, $file2Share->getShareOwner()); + // Expect permissions to not change + $this->assertEquals(\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_SHARE, $folder2Share->getPermissions()); + $this->assertEquals(\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_SHARE, $file1Share->getPermissions()); + $this->assertEquals(\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_SHARE, $subfolder1Share->getPermissions()); + $this->assertEquals(\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_SHARE, $file2Share->getPermissions()); + + // cleanup + $this->shareManager->deleteShare($folder1Share); + $this->shareManager->deleteShare($folder2Share); + $this->shareManager->deleteShare($file1Share); + $this->shareManager->deleteShare($subfolder1Share); + $this->shareManager->deleteShare($file2Share); + } } diff --git a/apps/sharebymail/lib/ShareByMailProvider.php b/apps/sharebymail/lib/ShareByMailProvider.php index ee14e354c5284..01dd3b428d179 100644 --- a/apps/sharebymail/lib/ShareByMailProvider.php +++ b/apps/sharebymail/lib/ShareByMailProvider.php @@ -75,7 +75,6 @@ * @package OCA\ShareByMail */ class ShareByMailProvider implements IShareProvider { - private IConfig $config; /** @var IDBConnection */ @@ -1159,7 +1158,7 @@ protected function getRawShare($id) { return $data; } - public function getSharesInFolder($userId, Folder $node, $reshares) { + public function getSharesInFolder($userId, Folder $node, $reshares, $shallow = true) { $qb = $this->dbConnection->getQueryBuilder(); $qb->select('*') ->from('share', 's') @@ -1185,8 +1184,13 @@ public function getSharesInFolder($userId, Folder $node, $reshares) { ); } - $qb->innerJoin('s', 'filecache' ,'f', $qb->expr()->eq('s.file_source', 'f.fileid')); - $qb->andWhere($qb->expr()->eq('f.parent', $qb->createNamedParameter($node->getId()))); + $qb->innerJoin('s', 'filecache', 'f', $qb->expr()->eq('s.file_source', 'f.fileid')); + + if ($shallow) { + $qb->andWhere($qb->expr()->eq('f.parent', $qb->createNamedParameter($node->getId()))); + } else { + $qb->andWhere($qb->expr()->like('f.path', $qb->createNamedParameter($this->dbConnection->escapeLikeParameter($node->getInternalPath()) . '/%'))); + } $qb->orderBy('id'); diff --git a/build/integration/sharing_features/sharing-v1-part3.feature b/build/integration/sharing_features/sharing-v1-part3.feature index 42e0f1b6add04..621aa95b364bc 100644 --- a/build/integration/sharing_features/sharing-v1-part3.feature +++ b/build/integration/sharing_features/sharing-v1-part3.feature @@ -514,6 +514,54 @@ Feature: sharing Then as "user1" the file "/shared/shared_file.txt" exists And as "user0" the file "/shared/shared_file.txt" exists + Scenario: Owner of subshares is adjusted after moving into received share + Given As an "admin" + And user "user0" exists + And user "user1" exists + And user "user2" exists + And user "user0" created a folder "/shared" + And folder "/shared" of user "user0" is shared with user "user1" + And user "user1" accepts last share + And user "user1" created a folder "/movein" + And user "user1" created a folder "/movein/subshare" + When As an "user1" + And folder "/movein" of user "user1" is shared with user "user2" + And save last share id + Then Getting info of last share + And Share fields of last share match with + | uid_file_owner | user1 | + | share_with | user2 | + When User "user1" moved file "/movein" to "/shared/movein" + Then As an "user0" + And Getting info of last share + And Share fields of last share match with + | uid_file_owner | user0 | + | share_with | user2 | + + Scenario: Owner of subshares is adjusted after moving out of received share + Given As an "admin" + And user "user0" exists + And user "user1" exists + And user "user2" exists + And user "user0" created a folder "/shared" + And user "user0" created a folder "/shared/moveout" + And user "user0" created a folder "/shared/moveout/subshare" + And folder "/shared" of user "user0" is shared with user "user1" + And user "user1" accepts last share + And As an "user1" + And folder "/shared/moveout/subshare" of user "user1" is shared with user "user2" + And save last share id + When As an "user1" + Then Getting info of last share + And Share fields of last share match with + | uid_file_owner | user0 | + | share_with | user2 | + When User "user1" moved file "/shared/moveout" to "/moveout" + Then Getting info of last share + And Share fields of last share match with + | uid_file_owner | user1 | + | share_with | user2 | + Scenario: Link shares inside of group shares keep their original data when the root share is updated Given As an "admin" And user "user0" exists diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php index 520bd17d3cf08..7719a1e6be3c1 100644 --- a/lib/private/Share20/DefaultShareProvider.php +++ b/lib/private/Share20/DefaultShareProvider.php @@ -641,9 +641,12 @@ public function move(\OCP\Share\IShare $share, $recipient) { return $share; } - public function getSharesInFolder($userId, Folder $node, $reshares) { + public function getSharesInFolder($userId, Folder $node, $reshares, $shallow = true) { $qb = $this->dbConn->getQueryBuilder(); - $qb->select('*') + $qb->select('s.*', + 'f.fileid', 'f.path', 'f.permissions AS f_permissions', 'f.storage', 'f.path_hash', + 'f.parent AS f_parent', 'f.name', 'f.mimetype', 'f.mimepart', 'f.size', 'f.mtime', 'f.storage_mtime', + 'f.encrypted', 'f.unencrypted_size', 'f.etag', 'f.checksum') ->from('share', 's') ->andWhere($qb->expr()->orX( $qb->expr()->eq('item_type', $qb->createNamedParameter('file')), @@ -679,12 +682,21 @@ public function getSharesInFolder($userId, Folder $node, $reshares) { }, $childMountNodes); $qb->innerJoin('s', 'filecache', 'f', $qb->expr()->eq('s.file_source', 'f.fileid')); - $qb->andWhere( - $qb->expr()->orX( - $qb->expr()->eq('f.parent', $qb->createNamedParameter($node->getId())), - $qb->expr()->in('f.fileid', $qb->createParameter('chunk')) - ) - ); + if ($shallow) { + $qb->andWhere( + $qb->expr()->orX( + $qb->expr()->eq('f.parent', $qb->createNamedParameter($node->getId())), + $qb->expr()->in('f.fileid', $qb->createParameter('chunk')) + ) + ); + } else { + $qb->andWhere( + $qb->expr()->orX( + $qb->expr()->like('f.path', $qb->createNamedParameter($this->dbConn->escapeLikeParameter($node->getInternalPath()) . '/%')), + $qb->expr()->in('f.fileid', $qb->createParameter('chunk')) + ) + ); + } $qb->orderBy('id'); diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 905a006372fc2..46f256df54d19 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -1303,11 +1303,11 @@ public function moveShare(IShare $share, $recipientId) { return $provider->move($share, $recipientId); } - public function getSharesInFolder($userId, Folder $node, $reshares = false) { + public function getSharesInFolder($userId, Folder $node, $reshares = false, $shallow = true) { $providers = $this->factory->getAllProviders(); - return array_reduce($providers, function ($shares, IShareProvider $provider) use ($userId, $node, $reshares) { - $newShares = $provider->getSharesInFolder($userId, $node, $reshares); + return array_reduce($providers, function ($shares, IShareProvider $provider) use ($userId, $node, $reshares, $shallow) { + $newShares = $provider->getSharesInFolder($userId, $node, $reshares, $shallow); foreach ($newShares as $fid => $data) { if (!isset($shares[$fid])) { $shares[$fid] = []; diff --git a/lib/public/Share/IManager.php b/lib/public/Share/IManager.php index f207ca87a2c98..0810acc673ac2 100644 --- a/lib/public/Share/IManager.php +++ b/lib/public/Share/IManager.php @@ -135,10 +135,11 @@ public function moveShare(IShare $share, $recipientId); * @param string $userId * @param Folder $node * @param bool $reshares + * @param bool $shallow Whether the method should stop at the first level, or look into sub-folders. * @return IShare[][] [$fileId => IShare[], ...] * @since 11.0.0 */ - public function getSharesInFolder($userId, Folder $node, $reshares = false); + public function getSharesInFolder($userId, Folder $node, $reshares = false, $shallow = true); /** * Get shares shared by (initiated) by the provided user. diff --git a/lib/public/Share/IShareProvider.php b/lib/public/Share/IShareProvider.php index 6af513360fe2a..c549592d6f629 100644 --- a/lib/public/Share/IShareProvider.php +++ b/lib/public/Share/IShareProvider.php @@ -123,10 +123,11 @@ public function move(\OCP\Share\IShare $share, $recipient); * @param string $userId * @param Folder $node * @param bool $reshares Also get the shares where $user is the owner instead of just the shares where $user is the initiator + * @param bool $shallow Whether the method should stop at the first level, or look into sub-folders. * @return \OCP\Share\IShare[][] * @since 11.0.0 */ - public function getSharesInFolder($userId, Folder $node, $reshares); + public function getSharesInFolder($userId, Folder $node, $reshares, $shallow = true); /** * Get all shares by the given user