From a5612458d66575e82ed5ea3f4ace0c0a89203d50 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 | 42 +------------- build/integration/features/bootstrap/Mail.php | 6 +- .../features/bootstrap/OccRunner.php | 57 +++++++++++++++++++ 3 files changed, 61 insertions(+), 44 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 76ab501a4b09b..a91e8ca6b2530 100644 --- a/build/integration/features/bootstrap/CommandLine.php +++ b/build/integration/features/bootstrap/CommandLine.php @@ -10,47 +10,7 @@ require __DIR__ . '/autoload.php'; trait CommandLine { - /** @var int return code of last command */ - private int $lastCode = 0; - /** @var string stdout of last command */ - private string $lastStdOut = ''; - /** @var string stderr of last command */ - private string $lastStdErr = ''; - protected string $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(array $args = [], string $inputString = ''): int { - $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..f3faab633588a --- /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 2c874446a3ee24a74424be4400a0b41c95cf1e4a 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 da6426abeba07..65934f15427eb 100644 --- a/build/integration/features/bootstrap/FeatureContext.php +++ b/build/integration/features/bootstrap/FeatureContext.php @@ -17,6 +17,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 5b7f976aaff28..3bc97ba0446e2 100644 --- a/build/integration/features/bootstrap/Sharing.php +++ b/build/integration/features/bootstrap/Sharing.php @@ -202,6 +202,26 @@ public function lastLinkShareCanBeDownloaded(): void { $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 217eb44aba7c7e689a22e582a0cb6678825a24e0 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 63045e79a826f..a395268aab770 100644 --- a/apps/files/lib/Service/OwnershipTransferService.php +++ b/apps/files/lib/Service/OwnershipTransferService.php @@ -20,6 +20,7 @@ use OCA\GroupFolders\Mount\GroupMountPoint; use OCP\Console\IOutput; use OCP\Console\Verbosity; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Encryption\IManager as IEncryptionManager; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Config\IHomeMountProvider; @@ -29,6 +30,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; @@ -55,6 +57,7 @@ public function __construct( private IFactory $l10nFactory, private IRootFolder $rootFolder, private IEventDispatcher $eventDispatcher, + private IDBConnection $connection, ) { } @@ -396,6 +399,9 @@ private function collectUsersShares( }, $shares))); } + /** + * @return array shares keyed by node ID + */ private function collectIncomingShares( string $sourceUid, IOutput $output, @@ -524,6 +530,8 @@ private function restoreShares( if ($shareMountPoint) { $this->mountManager->removeMount($shareMountPoint->getMountPoint()); } + + $this->promoteLinkShares($share); $this->shareManager->deleteShare($share); } else { if ($share->getShareOwner() === $sourceUid) { @@ -573,6 +581,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, @@ -604,11 +616,13 @@ private function transferIncomingShares( $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 @@ -628,8 +642,10 @@ private function transferIncomingShares( $this->mountManager->moveMount($oldMountPoint, $newMountPoint); 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); @@ -661,6 +677,21 @@ private function transferIncomingShares( $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(); + } + private function getShareMountPoint(string $uid, string $target): string { return '/' . $uid . '/files/' . trim($target, '/') . '/'; }