From 14c3f1ebd88389046aa5dbc971fcb2f00f8cd00f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Fri, 31 Jul 2020 11:10:48 +0200 Subject: [PATCH 1/8] Transfer shares of the transferred root node MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- .../lib/Service/OwnershipTransferService.php | 40 +++++++++++++++++-- lib/private/Files/Config/UserMountCache.php | 5 +++ lib/public/Files/Config/IUserMountCache.php | 7 ++++ 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/apps/files/lib/Service/OwnershipTransferService.php b/apps/files/lib/Service/OwnershipTransferService.php index 0b0a5f9abf77b..7da8c9b2bbf32 100644 --- a/apps/files/lib/Service/OwnershipTransferService.php +++ b/apps/files/lib/Service/OwnershipTransferService.php @@ -35,12 +35,14 @@ use OC\Files\View; use OCA\Files\Exception\TransferOwnershipException; use OCP\Encryption\IManager as IEncryptionManager; +use OCP\Files\Config\IUserMountCache; use OCP\Files\FileInfo; use OCP\Files\IHomeStorage; use OCP\Files\InvalidPathException; use OCP\Files\Mount\IMountManager; use OCP\IUser; use OCP\Share\IManager as IShareManager; +use OCP\Share\IShare; use Symfony\Component\Console\Helper\ProgressBar; use Symfony\Component\Console\Output\NullOutput; use Symfony\Component\Console\Output\OutputInterface; @@ -62,12 +64,17 @@ class OwnershipTransferService { /** @var IMountManager */ private $mountManager; + /** @var IUserMountCache */ + private $userMountCache; + public function __construct(IEncryptionManager $manager, IShareManager $shareManager, - IMountManager $mountManager) { + IMountManager $mountManager, + IUserMountCache $userMountCache) { $this->encryptionManager = $manager; $this->shareManager = $shareManager; $this->mountManager = $mountManager; + $this->userMountCache = $userMountCache; } /** @@ -148,7 +155,9 @@ public function transfer(IUser $sourceUser, // collect all the shares $shares = $this->collectUsersShares( $sourceUid, - $output + $output, + $view, + $sourcePath ); // transfer the files @@ -233,7 +242,9 @@ function (FileInfo $fileInfo) use ($progress) { } private function collectUsersShares(string $sourceUid, - OutputInterface $output): array { + OutputInterface $output, + View $view, + ?string $path = null): array { $output->writeln("Collecting all share information for files and folders of $sourceUid ..."); $shares = []; @@ -246,6 +257,23 @@ private function collectUsersShares(string $sourceUid, if (empty($sharePage)) { break; } + if ($path !== null) { + $sharePage = array_filter($sharePage, function (IShare $share) use ($view, $path) { + try { + $relativePath = $view->getPath($share->getNodeId()); + $singleFileTranfer = $view->is_file($path); + if ($singleFileTranfer) { + return Filesystem::normalizePath($relativePath) === Filesystem::normalizePath($path); + } + + return mb_strpos( + Filesystem::normalizePath($relativePath . '/', false), + Filesystem::normalizePath($path . '/', false)) === 0; + } catch (\Exception $e) { + return false; + } + }); + } $shares = array_merge($shares, $sharePage); $offset += 50; } @@ -306,6 +334,12 @@ private function restoreShares(string $sourceUid, $share->setSharedBy($destinationUid); } + + // trigger refetching of the node so that the new owner and mountpoint are taken into account + // otherwise the checks on the share update will fail due to the original node not being available in the new user scope + $this->userMountCache->clear(); + $share->setNodeId($share->getNode()->getId()); + $this->shareManager->updateShare($share); } } catch (\OCP\Files\NotFoundException $e) { diff --git a/lib/private/Files/Config/UserMountCache.php b/lib/private/Files/Config/UserMountCache.php index 9cf3b43a431b0..32bfd5a71f360 100644 --- a/lib/private/Files/Config/UserMountCache.php +++ b/lib/private/Files/Config/UserMountCache.php @@ -409,4 +409,9 @@ public function getUsedSpaceForUsers(array $users) { $result->closeCursor(); return $results; } + + public function clear(): void { + $this->cacheInfoCache = new CappedMemoryCache(); + $this->mountsForUsers = new CappedMemoryCache(); + } } diff --git a/lib/public/Files/Config/IUserMountCache.php b/lib/public/Files/Config/IUserMountCache.php index 9fca98dc8439c..fde4898bd39ae 100644 --- a/lib/public/Files/Config/IUserMountCache.php +++ b/lib/public/Files/Config/IUserMountCache.php @@ -117,4 +117,11 @@ public function remoteStorageMounts($storageId); * @since 13.0.0 */ public function getUsedSpaceForUsers(array $users); + + /** + * Clear all entries from the in-memory cache + * + * @since 20.0.0 + */ + public function clear(): void; } From 818b69ec545bf8408c421305eb77eb5234079e27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Mon, 7 Sep 2020 17:35:29 +0200 Subject: [PATCH 2/8] Transfer shares if no path provided MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- apps/files/lib/Service/OwnershipTransferService.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/files/lib/Service/OwnershipTransferService.php b/apps/files/lib/Service/OwnershipTransferService.php index 7da8c9b2bbf32..9d22c43b662ee 100644 --- a/apps/files/lib/Service/OwnershipTransferService.php +++ b/apps/files/lib/Service/OwnershipTransferService.php @@ -244,7 +244,7 @@ function (FileInfo $fileInfo) use ($progress) { private function collectUsersShares(string $sourceUid, OutputInterface $output, View $view, - ?string $path = null): array { + string $path): array { $output->writeln("Collecting all share information for files and folders of $sourceUid ..."); $shares = []; @@ -257,7 +257,7 @@ private function collectUsersShares(string $sourceUid, if (empty($sharePage)) { break; } - if ($path !== null) { + if ($path !== "$sourceUid/files") { $sharePage = array_filter($sharePage, function (IShare $share) use ($view, $path) { try { $relativePath = $view->getPath($share->getNodeId()); From bb8fe15a3b6e90614df8c5236bbefcfa3adad391 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 10 Sep 2020 04:18:53 +0200 Subject: [PATCH 3/8] Check whether file exists or not after transferring ownership MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The integration tests did not verify that the files were actually transferred between the users, only that the files were downloadable. Signed-off-by: Daniel Calviño Sánchez --- .../features/transfer-ownership.feature | 55 ++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/build/integration/features/transfer-ownership.feature b/build/integration/features/transfer-ownership.feature index a0959db6c1790..dc8197a12f490 100644 --- a/build/integration/features/transfer-ownership.feature +++ b/build/integration/features/transfer-ownership.feature @@ -9,6 +9,10 @@ Feature: transfer-ownership And As an "user1" And using received transfer folder of "user1" as dav path Then Downloaded content when downloading file "/somefile.txt" with range "bytes=0-6" should be "This is" + And using old dav path + And as "user0" the file "/somefile.txt" does not exist + And using received transfer folder of "user1" as dav path + And as "user1" the file "/somefile.txt" exists Scenario: transfering ownership of a folder Given user "user0" exists @@ -20,6 +24,10 @@ Feature: transfer-ownership And As an "user1" And using received transfer folder of "user1" as dav path Then Downloaded content when downloading file "/test/somefile.txt" with range "bytes=0-6" should be "This is" + And using old dav path + And as "user0" the folder "/test" does not exist + And using received transfer folder of "user1" as dav path + And as "user1" the folder "/test" exists Scenario: transfering ownership of file shares Given user "user0" exists @@ -32,6 +40,10 @@ Feature: transfer-ownership And the command was successful And As an "user2" Then Downloaded content when downloading file "/somefile.txt" with range "bytes=0-6" should be "This is" + And using old dav path + And as "user0" the file "/somefile.txt" does not exist + And using received transfer folder of "user1" as dav path + And as "user1" the file "/somefile.txt" exists Scenario: transfering ownership of folder shared with third user Given user "user0" exists @@ -45,6 +57,10 @@ Feature: transfer-ownership And the command was successful And As an "user2" Then Downloaded content when downloading file "/test/somefile.txt" with range "bytes=0-6" should be "This is" + And using old dav path + And as "user0" the folder "/test" does not exist + And using received transfer folder of "user1" as dav path + And as "user1" the folder "/test" exists Scenario: transfering ownership of folder shared with transfer recipient Given user "user0" exists @@ -59,6 +75,10 @@ Feature: transfer-ownership Then as "user1" the folder "/test" does not exist And using received transfer folder of "user1" as dav path And Downloaded content when downloading file "/test/somefile.txt" with range "bytes=0-6" should be "This is" + And using old dav path + And as "user0" the folder "/test" does not exist + And using received transfer folder of "user1" as dav path + And as "user1" the folder "/test" exists Scenario: transfering ownership of folder doubly shared with third user Given group "group1" exists @@ -76,6 +96,10 @@ Feature: transfer-ownership And the command was successful And As an "user2" Then Downloaded content when downloading file "/test/somefile.txt" with range "bytes=0-6" should be "This is" + And using old dav path + And as "user0" the folder "/test" does not exist + And using received transfer folder of "user1" as dav path + And as "user1" the folder "/test" exists Scenario: transfering ownership of file shares to user with the same id as the group Given user "user0" exists @@ -90,6 +114,10 @@ Feature: transfer-ownership And the command was successful And As an "user2" Then Downloaded content when downloading file "/somefile.txt" with range "bytes=0-6" should be "This is" + And using old dav path + And as "user0" the file "/somefile.txt" does not exist + And using received transfer folder of "user1" as dav path + And as "test" the file "/somefile.txt" exists Scenario: transfering ownership does not transfer received shares Given user "user0" exists @@ -103,6 +131,8 @@ Feature: transfer-ownership And As an "user1" And using received transfer folder of "user1" as dav path Then as "user1" the folder "/test" does not exist + And using old dav path + And as "user0" the folder "/test" exists @local_storage Scenario: transfering ownership does not transfer external storage @@ -148,6 +178,10 @@ Feature: transfer-ownership And As an "user1" And using received transfer folder of "user1" as dav path Then Downloaded content when downloading file "/test/somefile.txt" with range "bytes=0-6" should be "This is" + And using old dav path + And as "user0" the folder "/test" does not exist + And using received transfer folder of "user1" as dav path + And as "user1" the folder "/test" exists Scenario: transfering ownership of file shares Given user "user0" exists @@ -161,6 +195,10 @@ Feature: transfer-ownership And the command was successful And As an "user2" Then Downloaded content when downloading file "/somefile.txt" with range "bytes=0-6" should be "This is" + And using old dav path + And as "user0" the folder "/test" does not exist + And using received transfer folder of "user1" as dav path + And as "user1" the folder "/test" exists Scenario: transfering ownership of folder shared with third user Given user "user0" exists @@ -174,6 +212,10 @@ Feature: transfer-ownership And the command was successful And As an "user2" Then Downloaded content when downloading file "/test/somefile.txt" with range "bytes=0-6" should be "This is" + And using old dav path + And as "user0" the folder "/test" does not exist + And using received transfer folder of "user1" as dav path + And as "user1" the folder "/test" exists Scenario: transfering ownership of folder shared with transfer recipient Given user "user0" exists @@ -188,6 +230,10 @@ Feature: transfer-ownership Then as "user1" the folder "/test" does not exist And using received transfer folder of "user1" as dav path And Downloaded content when downloading file "/test/somefile.txt" with range "bytes=0-6" should be "This is" + And using old dav path + And as "user0" the folder "/test" does not exist + And using received transfer folder of "user1" as dav path + And as "user1" the folder "/test" exists Scenario: transfering ownership of folder doubly shared with third user Given group "group1" exists @@ -205,6 +251,10 @@ Feature: transfer-ownership And the command was successful And As an "user2" Then Downloaded content when downloading file "/test/somefile.txt" with range "bytes=0-6" should be "This is" + And using old dav path + And as "user0" the folder "/test" does not exist + And using received transfer folder of "user1" as dav path + And as "user1" the folder "/test" exists Scenario: transfering ownership does not transfer received shares Given user "user0" exists @@ -219,7 +269,10 @@ Feature: transfer-ownership And the command was successful And As an "user1" And using received transfer folder of "user1" as dav path - Then as "user1" the folder "/sub/test" does not exist + Then as "user1" the folder "/sub" exists + And as "user1" the folder "/sub/test" does not exist + And using old dav path + And as "user0" the folder "/sub" does not exist Scenario: transfering ownership does not transfer external storage Given user "user0" exists From 6d1374738ffc77bdc18362202a7811fa3860fa27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 10 Sep 2020 04:24:16 +0200 Subject: [PATCH 4/8] Check share ownership after transferring file ownership MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The integration tests did not verify that the shares were actually transferred between the users (or that they were removed due to being transferred to the sharee). Signed-off-by: Daniel Calviño Sánchez --- .../features/transfer-ownership.feature | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/build/integration/features/transfer-ownership.feature b/build/integration/features/transfer-ownership.feature index dc8197a12f490..7a1709c6be361 100644 --- a/build/integration/features/transfer-ownership.feature +++ b/build/integration/features/transfer-ownership.feature @@ -44,6 +44,13 @@ Feature: transfer-ownership And as "user0" the file "/somefile.txt" does not exist And using received transfer folder of "user1" as dav path And as "user1" the file "/somefile.txt" exists + And As an "user1" + And Getting info of last share + And the OCS status code should be "100" + And Share fields of last share match with + | uid_owner | user1 | + | uid_file_owner | user1 | + | share_with | user2 | Scenario: transfering ownership of folder shared with third user Given user "user0" exists @@ -61,6 +68,13 @@ Feature: transfer-ownership And as "user0" the folder "/test" does not exist And using received transfer folder of "user1" as dav path And as "user1" the folder "/test" exists + And As an "user1" + And Getting info of last share + And the OCS status code should be "100" + And Share fields of last share match with + | uid_owner | user1 | + | uid_file_owner | user1 | + | share_with | user2 | Scenario: transfering ownership of folder shared with transfer recipient Given user "user0" exists @@ -79,6 +93,8 @@ Feature: transfer-ownership And as "user0" the folder "/test" does not exist And using received transfer folder of "user1" as dav path And as "user1" the folder "/test" exists + And Getting info of last share + And the OCS status code should be "404" Scenario: transfering ownership of folder doubly shared with third user Given group "group1" exists @@ -100,6 +116,13 @@ Feature: transfer-ownership And as "user0" the folder "/test" does not exist And using received transfer folder of "user1" as dav path And as "user1" the folder "/test" exists + And As an "user1" + And Getting info of last share + And the OCS status code should be "100" + And Share fields of last share match with + | uid_owner | user1 | + | uid_file_owner | user1 | + | share_with | user2 | Scenario: transfering ownership of file shares to user with the same id as the group Given user "user0" exists @@ -118,6 +141,13 @@ Feature: transfer-ownership And as "user0" the file "/somefile.txt" does not exist And using received transfer folder of "user1" as dav path And as "test" the file "/somefile.txt" exists + And As an "test" + And Getting info of last share + And the OCS status code should be "100" + And Share fields of last share match with + | uid_owner | test | + | uid_file_owner | test | + | share_with | test | Scenario: transfering ownership does not transfer received shares Given user "user0" exists @@ -133,6 +163,13 @@ Feature: transfer-ownership Then as "user1" the folder "/test" does not exist And using old dav path And as "user0" the folder "/test" exists + And As an "user2" + And Getting info of last share + And the OCS status code should be "100" + And Share fields of last share match with + | uid_owner | user2 | + | uid_file_owner | user2 | + | share_with | user0 | @local_storage Scenario: transfering ownership does not transfer external storage @@ -199,6 +236,13 @@ Feature: transfer-ownership And as "user0" the folder "/test" does not exist And using received transfer folder of "user1" as dav path And as "user1" the folder "/test" exists + And As an "user1" + And Getting info of last share + And the OCS status code should be "100" + And Share fields of last share match with + | uid_owner | user1 | + | uid_file_owner | user1 | + | share_with | user2 | Scenario: transfering ownership of folder shared with third user Given user "user0" exists @@ -216,6 +260,13 @@ Feature: transfer-ownership And as "user0" the folder "/test" does not exist And using received transfer folder of "user1" as dav path And as "user1" the folder "/test" exists + And As an "user1" + And Getting info of last share + And the OCS status code should be "100" + And Share fields of last share match with + | uid_owner | user1 | + | uid_file_owner | user1 | + | share_with | user2 | Scenario: transfering ownership of folder shared with transfer recipient Given user "user0" exists @@ -234,6 +285,8 @@ Feature: transfer-ownership And as "user0" the folder "/test" does not exist And using received transfer folder of "user1" as dav path And as "user1" the folder "/test" exists + And Getting info of last share + And the OCS status code should be "404" Scenario: transfering ownership of folder doubly shared with third user Given group "group1" exists @@ -255,6 +308,13 @@ Feature: transfer-ownership And as "user0" the folder "/test" does not exist And using received transfer folder of "user1" as dav path And as "user1" the folder "/test" exists + And As an "user1" + And Getting info of last share + And the OCS status code should be "100" + And Share fields of last share match with + | uid_owner | user1 | + | uid_file_owner | user1 | + | share_with | user2 | Scenario: transfering ownership does not transfer received shares Given user "user0" exists @@ -273,6 +333,8 @@ Feature: transfer-ownership And as "user1" the folder "/sub/test" does not exist And using old dav path And as "user0" the folder "/sub" does not exist + And Getting info of last share + And the OCS status code should be "404" Scenario: transfering ownership does not transfer external storage Given user "user0" exists From 1375a42a84e461aef506b06eb714977ed6d9c67a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 10 Sep 2020 04:28:57 +0200 Subject: [PATCH 5/8] Add integration tests for transferring ownership of reshares MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently only transferring ownership of a reshare with a group to a user in the group is possible. When transferring ownership of a reshare with another user or with a group to a user not in the group restoring the share fails (but the command succeeds, it only fails for the specific files that are reshares). When transferring ownership of a path that is a reshare the command fails (as when a specific path is provided the path tries to move the file, it does not take into account reshares). The added integration tests reflect the above behaviours. Signed-off-by: Daniel Calviño Sánchez --- .../features/transfer-ownership.feature | 100 ++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/build/integration/features/transfer-ownership.feature b/build/integration/features/transfer-ownership.feature index 7a1709c6be361..c6b13fc9dc836 100644 --- a/build/integration/features/transfer-ownership.feature +++ b/build/integration/features/transfer-ownership.feature @@ -149,6 +149,91 @@ Feature: transfer-ownership | uid_file_owner | test | | share_with | test | + Scenario: transfering ownership of folder reshared with another user + Given user "user0" exists + And user "user1" exists + And user "user2" exists + And user "user3" exists + And User "user3" created a folder "/test" + And User "user3" uploads file "data/textfile.txt" to "/test/somefile.txt" + And folder "/test" of user "user3" is shared with user "user0" with permissions 31 + And user "user0" accepts last share + And folder "/test" of user "user0" is shared with user "user2" with permissions 31 + And user "user2" accepts last share + When transfering ownership from "user0" to "user1" + And the command was successful + And As an "user2" + Then Downloaded content when downloading file "/test/somefile.txt" with range "bytes=0-6" should be "This is" + And using old dav path + And as "user0" the folder "/test" exists + And using received transfer folder of "user1" as dav path + And as "user1" the folder "/test" does not exist + And As an "user0" + And Getting info of last share + And the OCS status code should be "100" + And Share fields of last share match with + | uid_owner | user0 | + | uid_file_owner | user3 | + | share_with | user2 | + + Scenario: transfering ownership of folder reshared with group to a user in the group + Given user "user0" exists + And user "user1" exists + And user "user2" exists + And user "user3" exists + And group "group1" exists + And user "user1" belongs to group "group1" + And User "user3" created a folder "/test" + And User "user3" uploads file "data/textfile.txt" to "/test/somefile.txt" + And folder "/test" of user "user3" is shared with user "user0" with permissions 31 + And user "user0" accepts last share + And folder "/test" of user "user0" is shared with group "group1" with permissions 31 + And user "user1" accepts last share + When transfering ownership from "user0" to "user1" + And the command was successful + And As an "user1" + Then Downloaded content when downloading file "/test/somefile.txt" with range "bytes=0-6" should be "This is" + And using old dav path + And as "user0" the folder "/test" exists + And using received transfer folder of "user1" as dav path + And as "user1" the folder "/test" does not exist + And As an "user1" + And Getting info of last share + And the OCS status code should be "100" + And Share fields of last share match with + | uid_owner | user1 | + | uid_file_owner | user3 | + | share_with | group1 | + + Scenario: transfering ownership of folder reshared with group to a user not in the group + Given user "user0" exists + And user "user1" exists + And user "user2" exists + And user "user3" exists + And group "group1" exists + And user "user2" belongs to group "group1" + And User "user3" created a folder "/test" + And User "user3" uploads file "data/textfile.txt" to "/test/somefile.txt" + And folder "/test" of user "user3" is shared with user "user0" with permissions 31 + And user "user0" accepts last share + And folder "/test" of user "user0" is shared with group "group1" with permissions 31 + And user "user2" accepts last share + When transfering ownership from "user0" to "user1" + And the command was successful + And As an "user2" + Then Downloaded content when downloading file "/test/somefile.txt" with range "bytes=0-6" should be "This is" + And using old dav path + And as "user0" the folder "/test" exists + And using received transfer folder of "user1" as dav path + And as "user1" the folder "/test" does not exist + And As an "user0" + And Getting info of last share + And the OCS status code should be "100" + And Share fields of last share match with + | uid_owner | user0 | + | uid_file_owner | user3 | + | share_with | group1 | + Scenario: transfering ownership does not transfer received shares Given user "user0" exists And user "user1" exists @@ -316,6 +401,21 @@ Feature: transfer-ownership | uid_file_owner | user1 | | share_with | user2 | + Scenario: transfering ownership of path fails for reshares + Given user "user0" exists + And user "user1" exists + And user "user2" exists + And user "user3" exists + And User "user3" created a folder "/test" + And User "user3" uploads file "data/textfile.txt" to "/test/somefile.txt" + And folder "/test" of user "user3" is shared with user "user0" with permissions 31 + And user "user0" accepts last share + And folder "/test" of user "user0" is shared with user "user2" with permissions 31 + And user "user2" accepts last share + When transfering ownership of path "test" from "user0" to "user1" + Then the command failed with exit code 1 + And the command error output contains the text "Could not transfer files." + Scenario: transfering ownership does not transfer received shares Given user "user0" exists And user "user1" exists From 29fcaa1cec5a632dab830e00248fe5d1faf8b02c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 18 Sep 2020 18:32:23 +0200 Subject: [PATCH 6/8] Add integration test for transferring the path of a single file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- .../features/transfer-ownership.feature | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/build/integration/features/transfer-ownership.feature b/build/integration/features/transfer-ownership.feature index c6b13fc9dc836..265e1d370e499 100644 --- a/build/integration/features/transfer-ownership.feature +++ b/build/integration/features/transfer-ownership.feature @@ -290,6 +290,20 @@ Feature: transfer-ownership Then the command error output contains the text "Unknown target user" And the command failed with exit code 1 + Scenario: transfering ownership of a file + Given user "user0" exists + And user "user1" exists + And User "user0" uploads file "data/textfile.txt" to "/somefile.txt" + When transfering ownership of path "somefile.txt" from "user0" to "user1" + And the command was successful + And As an "user1" + And using received transfer folder of "user1" as dav path + Then Downloaded content when downloading file "/somefile.txt" with range "bytes=0-6" should be "This is" + And using old dav path + And as "user0" the file "/somefile.txt" does not exist + And using received transfer folder of "user1" as dav path + And as "user1" the file "/somefile.txt" exists + Scenario: transfering ownership of a folder Given user "user0" exists And user "user1" exists From f400c44e8b7acbdf3fb15ec3dee4153656d46b8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 18 Sep 2020 18:32:59 +0200 Subject: [PATCH 7/8] Add integration tests for transferring files of a user with a risky name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The files:transfer-ownership performs a sanitization of users with "risky" display names (including characters like "\" or "/"). In order to allow (escaped) double quotes in the display name the regular expression used in the "user XXX with displayname YYY exists" step had to be adjusted. Signed-off-by: Daniel Calviño Sánchez --- .../features/bootstrap/CommandLineContext.php | 8 +++++ .../features/bootstrap/Provisioning.php | 2 +- .../features/transfer-ownership.feature | 32 +++++++++++++++++++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/build/integration/features/bootstrap/CommandLineContext.php b/build/integration/features/bootstrap/CommandLineContext.php index bf999fd205012..a689f0aab3089 100644 --- a/build/integration/features/bootstrap/CommandLineContext.php +++ b/build/integration/features/bootstrap/CommandLineContext.php @@ -26,6 +26,7 @@ require __DIR__ . '/../../vendor/autoload.php'; use Behat\Behat\Hook\Scope\BeforeScenarioScope; +use PHPUnit\Framework\Assert; class CommandLineContext implements \Behat\Behat\Context\Context { use CommandLine; @@ -128,4 +129,11 @@ public function usingTransferFolderAsDavPath($user) { $davPath = rtrim($davPath, '/') . $this->lastTransferPath; $this->featureContext->usingDavPath($davPath); } + + /** + * @Then /^transfer folder name contains "([^"]+)"$/ + */ + public function transferFolderNameContains($text) { + Assert::assertContains($text, $this->lastTransferPath); + } } diff --git a/build/integration/features/bootstrap/Provisioning.php b/build/integration/features/bootstrap/Provisioning.php index daf5b11569c01..31331092ae72b 100644 --- a/build/integration/features/bootstrap/Provisioning.php +++ b/build/integration/features/bootstrap/Provisioning.php @@ -70,7 +70,7 @@ public function assureUserExists($user) { } /** - * @Given /^user "([^"]*)" with displayname "([^"]*)" exists$/ + * @Given /^user "([^"]*)" with displayname "((?:[^"]|\\")*)" exists$/ * @param string $user */ public function assureUserWithDisplaynameExists($user, $displayname) { diff --git a/build/integration/features/transfer-ownership.feature b/build/integration/features/transfer-ownership.feature index 265e1d370e499..49326c96d93d0 100644 --- a/build/integration/features/transfer-ownership.feature +++ b/build/integration/features/transfer-ownership.feature @@ -29,6 +29,22 @@ Feature: transfer-ownership And using received transfer folder of "user1" as dav path And as "user1" the folder "/test" exists + Scenario: transfering ownership from user with risky display name + Given user "user0" with displayname "user0 \"risky\"? ヂspḷay 'na|\/|e':.#" exists + And user "user1" exists + And User "user0" created a folder "/test" + And User "user0" uploads file "data/textfile.txt" to "/test/somefile.txt" + When transfering ownership from "user0" to "user1" + And the command was successful + And As an "user1" + And using received transfer folder of "user1" as dav path + Then Downloaded content when downloading file "/test/somefile.txt" with range "bytes=0-6" should be "This is" + And transfer folder name contains "transferred from user0 -risky- ヂspḷay -na|-|e- on" + And using old dav path + And as "user0" the folder "/test" does not exist + And using received transfer folder of "user1" as dav path + And as "user1" the folder "/test" exists + Scenario: transfering ownership of file shares Given user "user0" exists And user "user1" exists @@ -319,6 +335,22 @@ Feature: transfer-ownership And using received transfer folder of "user1" as dav path And as "user1" the folder "/test" exists + Scenario: transfering ownership from user with risky display name + Given user "user0" with displayname "user0 \"risky\"? ヂspḷay 'na|\/|e':.#" exists + And user "user1" exists + And User "user0" created a folder "/test" + And User "user0" uploads file "data/textfile.txt" to "/test/somefile.txt" + When transfering ownership of path "test" from "user0" to "user1" + And the command was successful + And As an "user1" + And using received transfer folder of "user1" as dav path + Then Downloaded content when downloading file "/test/somefile.txt" with range "bytes=0-6" should be "This is" + And transfer folder name contains "transferred from user0 -risky- ヂspḷay -na|-|e- on" + And using old dav path + And as "user0" the folder "/test" does not exist + And using received transfer folder of "user1" as dav path + And as "user1" the folder "/test" exists + Scenario: transfering ownership of file shares Given user "user0" exists And user "user1" exists From 083d1dab1c3f06abae95dec52c63669a002a8914 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 18 Sep 2020 18:35:06 +0200 Subject: [PATCH 8/8] Add integration tests to check that only the given path is transferred MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Until recently (it was fixed in ac2999a26a) when a path was transferred other shares with the target user were removed, so a test was added to ensure that it does not happen again. Besides that a test to ensure that other files with the target user are not transferred was added too (it did not fail before, but seemed convenient to have that covered too :-) ). Signed-off-by: Daniel Calviño Sánchez --- .../features/transfer-ownership.feature | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/build/integration/features/transfer-ownership.feature b/build/integration/features/transfer-ownership.feature index 49326c96d93d0..b091d00dae8ff 100644 --- a/build/integration/features/transfer-ownership.feature +++ b/build/integration/features/transfer-ownership.feature @@ -351,6 +351,57 @@ Feature: transfer-ownership And using received transfer folder of "user1" as dav path And as "user1" the folder "/test" exists + Scenario: transfering ownership of path does not affect other files + Given user "user0" exists + And user "user1" exists + And User "user0" created a folder "/test" + And User "user0" uploads file "data/textfile.txt" to "/test/somefile.txt" + And User "user0" created a folder "/test2" + And User "user0" uploads file "data/textfile.txt" to "/test2/somefile.txt" + When transfering ownership of path "test" from "user0" to "user1" + And the command was successful + And As an "user1" + And using received transfer folder of "user1" as dav path + Then Downloaded content when downloading file "/test/somefile.txt" with range "bytes=0-6" should be "This is" + And using old dav path + And as "user0" the folder "/test" does not exist + And as "user0" the folder "/test2" exists + And as "user0" the file "/test2/somefile.txt" exists + And using received transfer folder of "user1" as dav path + And as "user1" the folder "/test" exists + And as "user1" the folder "/test2" does not exist + + Scenario: transfering ownership of path does not affect other shares + Given user "user0" exists + And user "user1" exists + And User "user0" created a folder "/test" + And User "user0" uploads file "data/textfile.txt" to "/test/somefile.txt" + And User "user0" created a folder "/test2" + And User "user0" uploads file "data/textfile.txt" to "/test2/sharedfile.txt" + And file "/test2/sharedfile.txt" of user "user0" is shared with user "user1" with permissions 19 + And user "user1" accepts last share + When transfering ownership of path "test" from "user0" to "user1" + And the command was successful + And As an "user1" + And using received transfer folder of "user1" as dav path + Then Downloaded content when downloading file "/test/somefile.txt" with range "bytes=0-6" should be "This is" + And using old dav path + And as "user0" the folder "/test" does not exist + And as "user0" the folder "/test2" exists + And as "user0" the file "/test2/sharedfile.txt" exists + And using received transfer folder of "user1" as dav path + And as "user1" the folder "/test" exists + And as "user1" the folder "/test2" does not exist + And using old dav path + And as "user1" the file "/sharedfile.txt" exists + And As an "user1" + And Getting info of last share + And the OCS status code should be "100" + And Share fields of last share match with + | uid_owner | user0 | + | uid_file_owner | user0 | + | share_with | user1 | + Scenario: transfering ownership of file shares Given user "user0" exists And user "user1" exists