diff --git a/apps/files_sharing/tests/CapabilitiesTest.php b/apps/files_sharing/tests/CapabilitiesTest.php index 644cde444bd32..a63e848a7fd2e 100644 --- a/apps/files_sharing/tests/CapabilitiesTest.php +++ b/apps/files_sharing/tests/CapabilitiesTest.php @@ -18,6 +18,7 @@ use OCP\IAppConfig; use OCP\IConfig; use OCP\IDateTimeZone; +use OCP\IDBConnection; use OCP\IGroupManager; use OCP\IURLGenerator; use OCP\IUserManager; @@ -100,6 +101,7 @@ private function getResults(array $map, array $typedMap = [], bool $federationEn $this->createMock(ShareDisableChecker::class), $this->createMock(IDateTimeZone::class), $appConfig, + $this->createMock(IDBConnection::class), ); $cap = new Capabilities($config, $appConfig, $shareManager, $appManager); diff --git a/apps/settings/tests/Settings/Admin/SharingTest.php b/apps/settings/tests/Settings/Admin/SharingTest.php index e506a0c4a436e..4d7601cc57acf 100644 --- a/apps/settings/tests/Settings/Admin/SharingTest.php +++ b/apps/settings/tests/Settings/Admin/SharingTest.php @@ -16,9 +16,11 @@ use OCP\IL10N; use OCP\IURLGenerator; use OCP\Share\IManager; +use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; +#[Group(name: 'DB')] class SharingTest extends TestCase { private Sharing $admin; diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php index 5300e6e1baad2..c7e4a4da581eb 100644 --- a/lib/private/Share20/DefaultShareProvider.php +++ b/lib/private/Share20/DefaultShareProvider.php @@ -987,7 +987,7 @@ public function getShareByToken($token) { * @return \OCP\Share\IShare * @throws InvalidShare */ - private function createShare($data) { + private function createShare($data): IShare { $share = new Share($this->rootFolder, $this->userManager); $share->setId((int)$data['id']) ->setShareType((int)$data['share_type']) diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index a8c7b827265f2..327d68278d6ad 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -14,6 +14,7 @@ use OCA\Files_Sharing\AppInfo\Application; use OCA\Files_Sharing\SharedStorage; use OCA\ShareByMail\ShareByMailProvider; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\File; @@ -27,6 +28,7 @@ use OCP\IAppConfig; use OCP\IConfig; use OCP\IDateTimeZone; +use OCP\IDBConnection; use OCP\IGroupManager; use OCP\IL10N; use OCP\IURLGenerator; @@ -86,6 +88,7 @@ public function __construct( private ShareDisableChecker $shareDisableChecker, private IDateTimeZone $dateTimeZone, private IAppConfig $appConfig, + private IDBConnection $connection, ) { $this->l = $this->l10nFactory->get('lib'); // The constructor of LegacyHooks registers the listeners of share events @@ -1068,35 +1071,76 @@ protected function promoteReshares(IShare $share): void { IShare::TYPE_EMAIL, ]; - foreach ($userIds as $userId) { - foreach ($shareTypes as $shareType) { + // Figure out which users has some shares with which providers + $qb = $this->connection->getQueryBuilder(); + $qb->select('uid_initiator', 'share_type', 'uid_owner', 'file_source') + ->from('share') + ->andWhere($qb->expr()->in('item_type', $qb->createNamedParameter(['file', 'folder'], IQueryBuilder::PARAM_STR_ARRAY))) + ->andWhere($qb->expr()->in('share_type', $qb->createNamedParameter($shareTypes, IQueryBuilder::PARAM_INT_ARRAY))) + ->andWhere( + $qb->expr()->orX( + $qb->expr()->in('uid_initiator', $qb->createNamedParameter($userIds, IQueryBuilder::PARAM_STR_ARRAY)), + // Special case for old shares created via the web UI + $qb->expr()->andX( + $qb->expr()->in('uid_owner', $qb->createNamedParameter($userIds, IQueryBuilder::PARAM_STR_ARRAY)), + $qb->expr()->isNull('uid_initiator') + ) + ) + ); + + if (!$node instanceof Folder) { + $qb->andWhere($qb->expr()->eq('file_source', $qb->createNamedParameter($node->getId(), IQueryBuilder::PARAM_INT))); + } + + $qb->orderBy('id'); + + $cursor = $qb->executeQuery(); + /** @var array> $rawShare */ + $rawShares = []; + while ($data = $cursor->fetch()) { + if (!isset($rawShares[$data['uid_initiator']])) { + $rawShares[$data['uid_initiator']] = []; + } + if (!in_array($data['share_type'], $rawShares[$data['uid_initiator']], true)) { + if ($node instanceof Folder) { + if ($data['file_source'] === null || $data['uid_owner'] === null) { + /* Ignore share of non-existing node */ + continue; + } + + // for federated shares the owner can be a remote user, in this + // case we use the initiator + if ($this->userManager->userExists($data['uid_owner'])) { + $userFolder = $this->rootFolder->getUserFolder($data['uid_owner']); + } else { + $userFolder = $this->rootFolder->getUserFolder($data['uid_initiator']); + } + $sharedNode = $userFolder->getFirstNodeById((int)$data['file_source']); + if (!$sharedNode) { + continue; + } + if ($node->getRelativePath($sharedNode->getPath()) !== null) { + $rawShares[$data['uid_initiator']][] = [(int)$data['share_type'], $sharedNode]; + } + } elseif ($node instanceof File) { + $rawShares[$data['uid_initiator']][] = [(int)$data['share_type'], $node]; + } + } + } + $cursor->closeCursor(); + + foreach ($rawShares as $userId => $shareInfos) { + foreach ($shareInfos as $shareInfo) { + [$shareType, $sharedNode] = $shareInfo; try { $provider = $this->factory->getProviderForType($shareType); - } catch (ProviderException $e) { + } catch (ProviderException) { continue; } - if ($node instanceof Folder) { - /* We need to get all shares by this user to get subshares */ - $shares = $provider->getSharesBy($userId, $shareType, null, false, -1, 0); - - foreach ($shares as $share) { - try { - $path = $share->getNode()->getPath(); - } catch (NotFoundException) { - /* Ignore share of non-existing node */ - continue; - } - if ($node->getRelativePath($path) !== null) { - /* If relative path is not null it means the shared node is the same or in a subfolder */ - $reshareRecords[] = $share; - } - } - } else { - $shares = $provider->getSharesBy($userId, $shareType, $node, false, -1, 0); - foreach ($shares as $child) { - $reshareRecords[] = $child; - } + $shares = $provider->getSharesBy($userId, $shareType, $sharedNode, false, -1, 0); + foreach ($shares as $child) { + $reshareRecords[] = $child; } } } diff --git a/tests/lib/Share20/LegacyHooksTest.php b/tests/lib/Share20/LegacyHooksTest.php index 2ce72b3fc1ccf..091bd5fb8dca7 100644 --- a/tests/lib/Share20/LegacyHooksTest.php +++ b/tests/lib/Share20/LegacyHooksTest.php @@ -9,7 +9,6 @@ use OC\EventDispatcher\EventDispatcher; use OC\Share20\LegacyHooks; -use OC\Share20\Manager; use OCP\Constants; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Cache\ICacheEntry; @@ -24,6 +23,7 @@ use OCP\Share\IManager as IShareManager; use OCP\Share\IShare; use OCP\Util; +use PHPUnit\Framework\Attributes\Group; use Psr\Log\LoggerInterface; use Test\TestCase; @@ -40,15 +40,11 @@ public function pre() { } } +#[Group(name: 'DB')] class LegacyHooksTest extends TestCase { - /** @var LegacyHooks */ - private $hooks; - - /** @var IEventDispatcher */ - private $eventDispatcher; - - /** @var Manager */ - private $manager; + private LegacyHooks $hooks; + private IEventDispatcher $eventDispatcher; + private IShareManager $manager; protected function setUp(): void { parent::setUp(); diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index e0abc86306fbf..be0d632287d2e 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -18,6 +18,9 @@ use OC\Share20\Share; use OC\Share20\ShareDisableChecker; use OCP\Constants; +use OCP\DB\IResult; +use OCP\DB\QueryBuilder\IExpressionBuilder; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\File; @@ -32,6 +35,7 @@ use OCP\IAppConfig; use OCP\IConfig; use OCP\IDateTimeZone; +use OCP\IDBConnection; use OCP\IGroup; use OCP\IGroupManager; use OCP\IL10N; @@ -138,6 +142,7 @@ protected function setUp(): void { $this->dispatcher = $this->createMock(IEventDispatcher::class); $this->userSession = $this->createMock(IUserSession::class); $this->knownUserService = $this->createMock(KnownUserService::class); + $this->connection = $this->createMock(IDBConnection::class); $this->shareDisabledChecker = new ShareDisableChecker($this->config, $this->userManager, $this->groupManager); $this->dateTimeZone = $this->createMock(IDateTimeZone::class); @@ -188,6 +193,7 @@ private function createManager(IProviderFactory $factory): Manager { $this->shareDisabledChecker, $this->dateTimeZone, $this->appConfig, + $this->connection, ); } @@ -216,6 +222,7 @@ private function createManagerMock() { $this->shareDisabledChecker, $this->dateTimeZone, $this->appConfig, + $this->connection, ]); } @@ -492,6 +499,7 @@ public function testPromoteReshareFile(): void { ->getMock(); $file = $this->createMock(File::class); + $file->method('getId')->willReturn(42); $share = $this->createMock(IShare::class); $share->method('getShareType')->willReturn(IShare::TYPE_USER); @@ -520,6 +528,33 @@ public function testPromoteReshareFile(): void { $manager->expects($this->exactly(1))->method('updateShare')->with($reShare); + $this->userManager->method('userExists')->willReturn(true); + $userFolder = $this->createMock(Folder::class); + $this->rootFolder->method('getUserFolder')->with('userA')->willReturn($userFolder); + $userFolder->method('getFirstNodeById') + ->with(42) + ->willReturn($file); + + $qb = $this->createMock(IQueryBuilder::class); + $result = $this->createMock(IResult::class); + $qb->method('select') + ->willReturn($qb); + $qb->method('from') + ->willReturn($qb); + $qb->method('andWhere') + ->willReturn($qb); + $qb->method('expr') + ->willReturn($this->createMock(IExpressionBuilder::class)); + $qb->method('executeQuery') + ->willReturn($result); + $this->connection->method('getQueryBuilder') + ->willReturn($qb); + $result->method('fetch') + ->willReturnOnConsecutiveCalls( + ['uid_initiator' => 'userB', 'share_type' => IShare::TYPE_USER, 'uid_owner' => 'userA', 'file_source' => 42], + false, + ); + self::invokePrivate($manager, 'promoteReshares', [$share]); } @@ -557,14 +592,18 @@ public function testPromoteReshare(): void { $reShareInOtherFolder->method('getNode')->willReturn($otherFolder); $this->defaultProvider->method('getSharesBy') - ->willReturnCallback(function ($userId, $shareType, $node, $reshares, $limit, $offset) use ($reShare, $reShareInSubFolder, $reShareInOtherFolder) { + ->willReturnCallback(function ($userId, $shareType, $node, $reshares, $limit, $offset) use ($folder, $subFolder, $reShare, $reShareInSubFolder, $reShareInOtherFolder) { if ($shareType === IShare::TYPE_USER) { - return match($userId) { - 'userB' => [$reShare,$reShareInSubFolder,$reShareInOtherFolder], - }; - } else { - return []; + if ($userId === 'userB') { + if ($node === $folder) { + return [$reShare]; + } + if ($node === $subFolder) { + return [$reShareInSubFolder]; + } + } } + $this->fail(); }); $manager->method('generalCreateChecks')->willThrowException(new GenericShareException()); @@ -579,6 +618,40 @@ public function testPromoteReshare(): void { $this->assertEquals($expected, $share); }); + $this->userManager->method('userExists')->willReturn(true); + $userFolder = $this->createMock(Folder::class); + $this->rootFolder->method('getUserFolder')->with('userA')->willReturn($userFolder); + $userFolder->method('getFirstNodeById') + ->willReturnCallback(function ($id) use ($subFolder, $otherFolder, $folder) { + return match ($id) { + 41 => $subFolder, + 42 => $otherFolder, + 43 => $folder, + }; + }); + + $qb = $this->createMock(IQueryBuilder::class); + $result = $this->createMock(IResult::class); + $qb->method('select') + ->willReturn($qb); + $qb->method('from') + ->willReturn($qb); + $qb->method('andWhere') + ->willReturn($qb); + $qb->method('expr') + ->willReturn($this->createMock(IExpressionBuilder::class)); + $qb->method('executeQuery') + ->willReturn($result); + $this->connection->method('getQueryBuilder') + ->willReturn($qb); + $result->method('fetch') + ->willReturnOnConsecutiveCalls( + ['uid_initiator' => 'userB', 'share_type' => IShare::TYPE_USER, 'uid_owner' => 'userA', 'file_source' => 41], + ['uid_initiator' => 'userB', 'share_type' => IShare::TYPE_USER, 'uid_owner' => 'userA', 'file_source' => 42], + ['uid_initiator' => 'userB', 'share_type' => IShare::TYPE_USER, 'uid_owner' => 'userA', 'file_source' => 43], + false, + ); + self::invokePrivate($manager, 'promoteReshares', [$share]); } @@ -607,6 +680,33 @@ public function testPromoteReshareWhenUserHasAnotherShare(): void { /* No share is promoted because generalCreateChecks does not throw */ $manager->expects($this->never())->method('updateShare'); + $this->userManager->method('userExists')->willReturn(true); + $userFolder = $this->createMock(Folder::class); + $this->rootFolder->method('getUserFolder')->with('userA')->willReturn($userFolder); + $userFolder->method('getFirstNodeById') + ->with(42) + ->willReturn($folder); + + $qb = $this->createMock(IQueryBuilder::class); + $result = $this->createMock(IResult::class); + $qb->method('select') + ->willReturn($qb); + $qb->method('from') + ->willReturn($qb); + $qb->method('andWhere') + ->willReturn($qb); + $qb->method('expr') + ->willReturn($this->createMock(IExpressionBuilder::class)); + $qb->method('executeQuery') + ->willReturn($result); + $this->connection->method('getQueryBuilder') + ->willReturn($qb); + $result->method('fetch') + ->willReturnOnConsecutiveCalls( + ['uid_initiator' => 'userB', 'share_type' => IShare::TYPE_USER, 'uid_owner' => 'userA', 'file_source' => 42], + false, + ); + self::invokePrivate($manager, 'promoteReshares', [$share]); } @@ -662,6 +762,13 @@ public function testPromoteReshareOfUsersInGroupShare(): void { $manager->method('getSharedWith')->willReturn([]); + $this->userManager->method('userExists')->willReturn(true); + $userFolder = $this->createMock(Folder::class); + $this->rootFolder->method('getUserFolder')->with('userA')->willReturn($userFolder); + $userFolder->method('getFirstNodeById') + ->with(42) + ->willReturn($folder); + $calls = [ $reShare1, $reShare2, @@ -673,6 +780,27 @@ public function testPromoteReshareOfUsersInGroupShare(): void { $this->assertEquals($expected, $share); }); + $qb = $this->createMock(IQueryBuilder::class); + $result = $this->createMock(IResult::class); + $qb->method('select') + ->willReturn($qb); + $qb->method('from') + ->willReturn($qb); + $qb->method('andWhere') + ->willReturn($qb); + $qb->method('expr') + ->willReturn($this->createMock(IExpressionBuilder::class)); + $qb->method('executeQuery') + ->willReturn($result); + $this->connection->method('getQueryBuilder') + ->willReturn($qb); + $result->method('fetch') + ->willReturnOnConsecutiveCalls( + ['uid_initiator' => 'userB', 'share_type' => IShare::TYPE_USER, 'uid_owner' => 'userA', 'file_source' => 42], + ['uid_initiator' => 'userC', 'share_type' => IShare::TYPE_USER, 'uid_owner' => 'userA', 'file_source' => 42], + false, + ); + self::invokePrivate($manager, 'promoteReshares', [$share]); }