diff --git a/apps/files/lib/Service/OwnershipTransferService.php b/apps/files/lib/Service/OwnershipTransferService.php index 5e536ffa48fb6..6d75913132db6 100644 --- a/apps/files/lib/Service/OwnershipTransferService.php +++ b/apps/files/lib/Service/OwnershipTransferService.php @@ -18,6 +18,7 @@ use OCA\Files\Exception\TransferOwnershipException; use OCA\Files_External\Config\ConfigAdapter; use OCA\GroupFolders\Mount\GroupMountPoint; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Encryption\IManager as IEncryptionManager; use OCP\Files\Config\IHomeMountProvider; use OCP\Files\Config\IUserMountCache; @@ -27,6 +28,7 @@ use OCP\Files\IRootFolder; use OCP\Files\Mount\IMountManager; use OCP\Files\NotFoundException; +use OCP\IDBConnection; use OCP\IUser; use OCP\IUserManager; use OCP\L10N\IFactory; @@ -53,6 +55,7 @@ public function __construct( private IUserManager $userManager, private IFactory $l10nFactory, private IRootFolder $rootFolder, + private IDBConnection $connection, ) { } @@ -388,6 +391,9 @@ private function collectUsersShares( }, $shares))); } + /** + * @return array shares keyed by node ID + */ private function collectIncomingShares( string $sourceUid, OutputInterface $output, @@ -517,6 +523,8 @@ private function restoreShares( if ($shareMountPoint) { $this->mountManager->removeMount($shareMountPoint->getMountPoint()); } + + $this->promoteLinkShares($share); $this->shareManager->deleteShare($share); } else { if ($share->getShareOwner() === $sourceUid) { @@ -565,6 +573,10 @@ private function restoreShares( $output->writeln(''); } + /** + * @param array $sourceShares shares of the source user, keyed by node ID + * @param array $destinationShares shares of the destination user, keyed by node ID + */ private function transferIncomingShares(string $sourceUid, string $destinationUid, array $sourceShares, @@ -594,11 +606,13 @@ private function transferIncomingShares(string $sourceUid, $shareTarget = $finalShareTarget . $shareTarget; if ($share->getShareType() === IShare::TYPE_USER && $share->getSharedBy() === $destinationUid) { + $this->promoteLinkShares($share); $this->shareManager->deleteShare($share); } elseif (isset($destinationShares[$share->getNodeId()])) { $destinationShare = $destinationShares[$share->getNodeId()]; // Keep the share which has the most permissions and discard the other one. if ($destinationShare->getPermissions() < $share->getPermissions()) { + $this->promoteLinkShares($destinationShare, $share->getId()); $this->shareManager->deleteShare($destinationShare); $share->setSharedWith($destinationUid); // trigger refetching of the node so that the new owner and mountpoint are taken into account @@ -615,8 +629,10 @@ private function transferIncomingShares(string $sourceUid, $this->shareManager->moveShare($share, $destinationUid); continue; } + $this->promoteLinkShares($share, $destinationShare->getId()); $this->shareManager->deleteShare($share); } elseif ($share->getShareOwner() === $destinationUid) { + $this->promoteLinkShares($share); $this->shareManager->deleteShare($share); } else { $share->setSharedWith($destinationUid); @@ -644,4 +660,19 @@ private function transferIncomingShares(string $sourceUid, $progress->finish(); $output->writeln(''); } + + /** + * Sets the parent column of the link/email shares with $share as parent to + * `null` or, if provided, to $newParentId. + */ + private function promoteLinkShares(IShare $share, ?string $newParentId = null): void { + $qb = $this->connection->getQueryBuilder(); + $parentParam = $newParentId === null ? $qb->createNamedParameter(null, IQueryBuilder::PARAM_NULL) : $qb->createNamedParameter($newParentId, IQueryBuilder::PARAM_STR); + + $qb->update('share') + ->set('parent', $parentParam) + ->where($qb->expr()->eq('parent', $qb->createNamedParameter($share->getId()))) + ->andWhere($qb->expr()->in('share_type', $qb->createNamedParameter([IShare::TYPE_LINK, IShare::TYPE_EMAIL], IQueryBuilder::PARAM_INT_ARRAY))) + ->executeStatement(); + } } diff --git a/build/integration/features/bootstrap/CommandLine.php b/build/integration/features/bootstrap/CommandLine.php index 0ffa1b9623d4c..a9ec31980cfd3 100644 --- a/build/integration/features/bootstrap/CommandLine.php +++ b/build/integration/features/bootstrap/CommandLine.php @@ -10,49 +10,7 @@ require __DIR__ . '/../../vendor/autoload.php'; trait CommandLine { - /** @var int return code of last command */ - private $lastCode; - /** @var string stdout of last command */ - private $lastStdOut; - /** @var string stderr of last command */ - private $lastStdErr; - - /** @var string */ - protected $ocPath = '../..'; - - /** - * Invokes an OCC command - * - * @param []string $args OCC command, the part behind "occ". For example: "files:transfer-ownership" - * @return int exit code - */ - public function runOcc($args = [], string $inputString = '') { - $args = array_map(function ($arg) { - return escapeshellarg($arg); - }, $args); - $args[] = '--no-ansi'; - $args = implode(' ', $args); - - $descriptor = [ - 0 => ['pipe', 'r'], - 1 => ['pipe', 'w'], - 2 => ['pipe', 'w'], - ]; - $process = proc_open('php console.php ' . $args, $descriptor, $pipes, $this->ocPath); - if ($inputString !== '') { - fwrite($pipes[0], $inputString . "\n"); - fclose($pipes[0]); - } - $this->lastStdOut = stream_get_contents($pipes[1]); - $this->lastStdErr = stream_get_contents($pipes[2]); - $this->lastCode = proc_close($process); - - // Clean opcode cache - $client = new GuzzleHttp\Client(); - $client->request('GET', 'http://localhost:8080/apps/testing/clean_opcode_cache.php'); - - return $this->lastCode; - } + use OccRunner; /** * @Given /^invoking occ with "([^"]*)"$/ diff --git a/build/integration/features/bootstrap/FeatureContext.php b/build/integration/features/bootstrap/FeatureContext.php index ec00c79016014..865c4a0fd9501 100644 --- a/build/integration/features/bootstrap/FeatureContext.php +++ b/build/integration/features/bootstrap/FeatureContext.php @@ -15,6 +15,7 @@ */ class FeatureContext implements Context, SnippetAcceptingContext { use AppConfiguration; + use OccRunner; use ContactsMenu; use ExternalStorage; use Search; diff --git a/build/integration/features/bootstrap/Mail.php b/build/integration/features/bootstrap/Mail.php index d48ed6399c521..4c7ca6a0d27e9 100644 --- a/build/integration/features/bootstrap/Mail.php +++ b/build/integration/features/bootstrap/Mail.php @@ -5,7 +5,7 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ trait Mail { - // CommandLine trait is expected to be used in the class that uses this + // OccRunner trait is expected to be used in the class that uses this // trait. /** @@ -23,7 +23,7 @@ public function killDummyMailServer() { exec('kill ' . $this->fakeSmtpServerPid); - $this->invokingTheCommand('config:system:delete mail_smtpport'); + $this->runOcc(['config:system:delete', 'mail_smtpport']); } /** @@ -32,7 +32,7 @@ public function killDummyMailServer() { public function dummyMailServerIsListening() { // Default smtpport (25) is restricted for regular users, so the // FakeSMTP uses 2525 instead. - $this->invokingTheCommand('config:system:set mail_smtpport --value=2525 --type integer'); + $this->runOcc(explode(' ', 'config:system:set mail_smtpport --value=2525 --type integer')); $this->fakeSmtpServerPid = exec('php features/bootstrap/FakeSMTPHelper.php >/dev/null 2>&1 & echo $!'); } diff --git a/build/integration/features/bootstrap/OccRunner.php b/build/integration/features/bootstrap/OccRunner.php new file mode 100644 index 0000000000000..6365571718ef0 --- /dev/null +++ b/build/integration/features/bootstrap/OccRunner.php @@ -0,0 +1,57 @@ + ['pipe', 'r'], + 1 => ['pipe', 'w'], + 2 => ['pipe', 'w'], + ]; + $process = proc_open('php console.php ' . $args, $descriptor, $pipes, $this->ocPath); + if ($inputString !== '') { + fwrite($pipes[0], $inputString . "\n"); + fclose($pipes[0]); + } + $this->lastStdOut = stream_get_contents($pipes[1]); + $this->lastStdErr = stream_get_contents($pipes[2]); + $this->lastCode = proc_close($process); + + // Clean opcode cache + $client = new GuzzleHttp\Client(); + $client->request('GET', 'http://localhost:8080/apps/testing/clean_opcode_cache.php'); + + return $this->lastCode; + } +} diff --git a/build/integration/features/bootstrap/Sharing.php b/build/integration/features/bootstrap/Sharing.php index c2c2fcda939b9..f0c8ed725a566 100644 --- a/build/integration/features/bootstrap/Sharing.php +++ b/build/integration/features/bootstrap/Sharing.php @@ -152,6 +152,26 @@ public function lastLinkShareCanBeDownloaded() { $this->checkDownload($fullUrl, null, 'text/plain'); } + /** + * @Then /^last link share can not be downloaded with status "([^"]*)"$/ + */ + public function lastLinkShareCanNotBeDownloaded(string $status): void { + if (count($this->lastShareData->data->element) > 0) { + $url = $this->lastShareData->data[0]->url; + } else { + $url = $this->lastShareData->data->url; + } + $fullUrl = $url . '/download'; + + $client = new Client(); + try { + $response = $client->get($fullUrl, ['stream' => true]); + } catch (ClientException $ex) { + $response = $ex->getResponse(); + } + Assert::assertNotEquals($status, $response->getStatusCode()); + } + /** * @Then /^last share can be downloaded$/ */ diff --git a/build/integration/files_features/transfer-ownership.feature b/build/integration/files_features/transfer-ownership.feature index 6f7a79441662e..28ffb9982771d 100644 --- a/build/integration/files_features/transfer-ownership.feature +++ b/build/integration/files_features/transfer-ownership.feature @@ -252,6 +252,95 @@ Feature: transfer-ownership | uid_file_owner | user3 | | share_with | group1 | + Scenario: transferring ownership of folder shared with transfer recipient who created a link share + 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 folder "/test" of user "user0" is shared with user "user1" with permissions 31 + And user "user1" accepts last share + And as "user1" creating a share with + | path | test/somefile.txt | + | shareType | 3 | + And the OCS status code should be "100" + When transferring ownership from "user0" to "user1" + And the command was successful + Then last link share can be downloaded + + Scenario: transferring ownership of folder shared with transfer recipient who created a mail share + Given dummy mail server is listening + And 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 folder "/test" of user "user0" is shared with user "user1" with permissions 31 + And user "user1" accepts last share + And as "user1" creating a share with + | path | test/somefile.txt | + | shareType | 4 | + | shareWith | dumy@test.com | + And the OCS status code should be "100" + When transferring ownership from "user0" to "user1" + And the command was successful + Then last share can be downloaded + + Scenario: transferring ownership of folder owned by transfer recipient with a link share created by the source + Given user "user0" exists + And user "user1" exists + And user "user2" exists + And User "user1" created a folder "/test" + And User "user1" uploads file "data/textfile.txt" to "/test/somefile.txt" + And folder "/test" of user "user1" is shared with user "user2" with permissions 31 + And user "user2" accepts last share + And folder "/test" of user "user2" is shared with user "user0" with permissions 31 + And user "user0" accepts last share + And as "user0" creating a share with + | path | test/somefile.txt | + | shareType | 3 | + And the OCS status code should be "100" + When transferring ownership from "user0" to "user1" + And the command was successful + Then last link share can be downloaded + + Scenario: transferring ownership of folder shared directly by transfer recipient with a link share created by the source + Given user "user0" exists + And user "user1" exists + And User "user1" created a folder "/test" + And User "user1" uploads file "data/textfile.txt" to "/test/somefile.txt" + And folder "/test" of user "user1" is shared with user "user0" with permissions 31 + And user "user0" accepts last share + And as "user0" creating a share with + | path | test/somefile.txt | + | shareType | 3 | + And the OCS status code should be "100" + When transferring ownership from "user0" to "user1" + And the command was successful + Then last link share can be downloaded + + Scenario: transferring ownership of folder shared with source and destination independently with a link share created by destination + Given user "user0" exists + And user "user1" exists + And user "user2" exists + And User "user2" created a folder "/test" + And User "user2" uploads file "data/textfile.txt" to "/test/somefile.txt" + And folder "/test" of user "user2" is shared with user "user0" with permissions 31 + And user "user0" accepts last share + And save the last share data as "survivor" + And folder "/test" of user "user2" is shared with user "user1" with permissions 17 + And user "user1" accepts last share + And as "user1" creating a share with + | path | test/somefile.txt | + | shareType | 3 | + And the OCS status code should be "100" + And save the last share data as "link" + When transferring ownership from "user0" to "user1" + And the command was successful + Then last link share can be downloaded + When restore the last share data from "survivor" + And Deleting last share + And restore the last share data from "link" + Then last link share can not be downloaded with status "404" + Scenario: transferring ownership transfers received shares Given user "user0" exists And user "user1" exists