From 49c8ccce84baec8afe5204e19865ecec560c4aa0 Mon Sep 17 00:00:00 2001 From: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> Date: Fri, 11 Sep 2026 12:34:58 +0200 Subject: [PATCH 1/3] test(integration): split CommandLine for reuse CommandLine defines both logic to run occ commands in integration tests as wel as behat steps. This is problematic as if two contexts are loaded and both need the ability to run occ commands, they will cause a conflict due to the steps being defined more than once. Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> --- .../features/bootstrap/CommandLine.php | 44 +------------- build/integration/features/bootstrap/Mail.php | 6 +- .../features/bootstrap/OccRunner.php | 57 +++++++++++++++++++ 3 files changed, 61 insertions(+), 46 deletions(-) create mode 100644 build/integration/features/bootstrap/OccRunner.php 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/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; + } +} From 88bc61cc1f3cb5f18a01d15398a08dd96e38e0ff Mon Sep 17 00:00:00 2001 From: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> Date: Fri, 11 Sep 2026 12:37:15 +0200 Subject: [PATCH 2/3] test(integration): add cases in ownership transfer tests for link shares Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> --- .../features/bootstrap/FeatureContext.php | 1 + .../features/bootstrap/Sharing.php | 20 +++++ .../files_features/transfer-ownership.feature | 89 +++++++++++++++++++ 3 files changed, 110 insertions(+) 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/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 From 050683bc16529841b906ee91d1261762eabcf419 Mon Sep 17 00:00:00 2001 From: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> Date: Fri, 11 Sep 2026 12:38:50 +0200 Subject: [PATCH 3/3] fix: preserve link shares on ownership transfer Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> --- .../lib/Service/OwnershipTransferService.php | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) 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(); + } }