From d6b880faa8f60554a309e0886ee3db5628ea03f2 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 a62386af2f30edd2fff012588e6917a8cd9a9822 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 6ce87b208be56..41a9ef759171e 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 2ad1eb6f7443b..10f5fe50772d3 100644 --- a/build/integration/features/bootstrap/Sharing.php +++ b/build/integration/features/bootstrap/Sharing.php @@ -194,6 +194,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 1b6326ee738aa5cfb0653881171998f58b6592d0 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 3864710ba3d88..5cb20b9989ecb 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\EventDispatcher\IEventDispatcher; use OCP\Files\Config\IHomeMountProvider; @@ -28,6 +29,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; @@ -56,6 +58,7 @@ public function __construct( private IFactory $l10nFactory, private IRootFolder $rootFolder, private IEventDispatcher $eventDispatcher, + private IDBConnection $connection, ) { } @@ -399,6 +402,9 @@ private function collectUsersShares( }, $shares))); } + /** + * @return array shares keyed by node ID + */ private function collectIncomingShares( string $sourceUid, OutputInterface $output, @@ -528,6 +534,8 @@ private function restoreShares( if ($shareMountPoint) { $this->mountManager->removeMount($shareMountPoint->getMountPoint()); } + + $this->promoteLinkShares($share); $this->shareManager->deleteShare($share); } else { if ($share->getShareOwner() === $sourceUid) { @@ -577,6 +585,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, @@ -608,11 +620,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 @@ -632,8 +646,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); @@ -665,6 +681,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, '/') . '/'; }