From ffa6784bba38638f813bd92fadfb15c4d1a65e2f Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 18 Dec 2025 13:34:00 +0100 Subject: [PATCH] feat: perform share mount validation on share instead of on mount Signed-off-by: Robin Appelman --- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + .../files_sharing/lib/AppInfo/Application.php | 11 + .../lib/Listener/SharesUpdatedListener.php | 66 ++++++ apps/files_sharing/lib/MountProvider.php | 30 ++- .../lib/ShareTargetValidator.php | 9 +- apps/files_sharing/lib/SharedMount.php | 6 +- apps/files_sharing/tests/ApiTest.php | 2 + .../tests/ShareTargetValidatorTest.php | 141 +++++++++++++ apps/files_sharing/tests/SharedMountTest.php | 191 +----------------- .../files_sharing/tests/SharedStorageTest.php | 97 --------- apps/files_versions/tests/VersioningTest.php | 2 + .../sharing_features/sharing-v1-part2.feature | 12 +- .../sharing_features/sharing-v1.feature | 2 + lib/private/Files/Config/UserMountCache.php | 9 + tests/lib/TestCase.php | 3 + 16 files changed, 270 insertions(+), 313 deletions(-) create mode 100644 apps/files_sharing/lib/Listener/SharesUpdatedListener.php create mode 100644 apps/files_sharing/tests/ShareTargetValidatorTest.php diff --git a/apps/files_sharing/composer/composer/autoload_classmap.php b/apps/files_sharing/composer/composer/autoload_classmap.php index 6750f63164d..0ba6ba1b816 100644 --- a/apps/files_sharing/composer/composer/autoload_classmap.php +++ b/apps/files_sharing/composer/composer/autoload_classmap.php @@ -70,6 +70,7 @@ return array( 'OCA\\Files_Sharing\\Listener\\LoadPublicFileRequestAuthListener' => $baseDir . '/../lib/Listener/LoadPublicFileRequestAuthListener.php', 'OCA\\Files_Sharing\\Listener\\LoadSidebarListener' => $baseDir . '/../lib/Listener/LoadSidebarListener.php', 'OCA\\Files_Sharing\\Listener\\ShareInteractionListener' => $baseDir . '/../lib/Listener/ShareInteractionListener.php', + 'OCA\\Files_Sharing\\Listener\\SharesUpdatedListener' => $baseDir . '/../lib/Listener/SharesUpdatedListener.php', 'OCA\\Files_Sharing\\Listener\\UserAddedToGroupListener' => $baseDir . '/../lib/Listener/UserAddedToGroupListener.php', 'OCA\\Files_Sharing\\Listener\\UserShareAcceptanceListener' => $baseDir . '/../lib/Listener/UserShareAcceptanceListener.php', 'OCA\\Files_Sharing\\Middleware\\OCSShareAPIMiddleware' => $baseDir . '/../lib/Middleware/OCSShareAPIMiddleware.php', diff --git a/apps/files_sharing/composer/composer/autoload_static.php b/apps/files_sharing/composer/composer/autoload_static.php index 922bc6d7fd3..03906cda047 100644 --- a/apps/files_sharing/composer/composer/autoload_static.php +++ b/apps/files_sharing/composer/composer/autoload_static.php @@ -85,6 +85,7 @@ class ComposerStaticInitFiles_Sharing 'OCA\\Files_Sharing\\Listener\\LoadPublicFileRequestAuthListener' => __DIR__ . '/..' . '/../lib/Listener/LoadPublicFileRequestAuthListener.php', 'OCA\\Files_Sharing\\Listener\\LoadSidebarListener' => __DIR__ . '/..' . '/../lib/Listener/LoadSidebarListener.php', 'OCA\\Files_Sharing\\Listener\\ShareInteractionListener' => __DIR__ . '/..' . '/../lib/Listener/ShareInteractionListener.php', + 'OCA\\Files_Sharing\\Listener\\SharesUpdatedListener' => __DIR__ . '/..' . '/../lib/Listener/SharesUpdatedListener.php', 'OCA\\Files_Sharing\\Listener\\UserAddedToGroupListener' => __DIR__ . '/..' . '/../lib/Listener/UserAddedToGroupListener.php', 'OCA\\Files_Sharing\\Listener\\UserShareAcceptanceListener' => __DIR__ . '/..' . '/../lib/Listener/UserShareAcceptanceListener.php', 'OCA\\Files_Sharing\\Middleware\\OCSShareAPIMiddleware' => __DIR__ . '/..' . '/../lib/Middleware/OCSShareAPIMiddleware.php', diff --git a/apps/files_sharing/lib/AppInfo/Application.php b/apps/files_sharing/lib/AppInfo/Application.php index 084c33b5fed..9701191f018 100644 --- a/apps/files_sharing/lib/AppInfo/Application.php +++ b/apps/files_sharing/lib/AppInfo/Application.php @@ -14,6 +14,7 @@ use OCA\Files\Event\LoadAdditionalScriptsEvent; use OCA\Files\Event\LoadSidebar; use OCA\Files_Sharing\Capabilities; use OCA\Files_Sharing\Config\ConfigLexicon; +use OCA\Files_Sharing\Event\UserShareAccessUpdatedEvent; use OCA\Files_Sharing\External\Manager; use OCA\Files_Sharing\External\MountProvider as ExternalMountProvider; use OCA\Files_Sharing\Helper; @@ -24,6 +25,7 @@ use OCA\Files_Sharing\Listener\LoadAdditionalListener; use OCA\Files_Sharing\Listener\LoadPublicFileRequestAuthListener; use OCA\Files_Sharing\Listener\LoadSidebarListener; use OCA\Files_Sharing\Listener\ShareInteractionListener; +use OCA\Files_Sharing\Listener\SharesUpdatedListener; use OCA\Files_Sharing\Listener\UserAddedToGroupListener; use OCA\Files_Sharing\Listener\UserShareAcceptanceListener; use OCA\Files_Sharing\Middleware\OCSShareAPIMiddleware; @@ -49,9 +51,11 @@ use OCP\Files\Events\Node\BeforeNodeReadEvent; use OCP\Group\Events\GroupChangedEvent; use OCP\Group\Events\GroupDeletedEvent; use OCP\Group\Events\UserAddedEvent; +use OCP\Group\Events\UserRemovedEvent; use OCP\IConfig; use OCP\IDBConnection; use OCP\IGroup; +use OCP\Share\Events\BeforeShareDeletedEvent; use OCP\Share\Events\ShareCreatedEvent; use OCP\User\Events\UserChangedEvent; use OCP\User\Events\UserDeletedEvent; @@ -111,6 +115,13 @@ class Application extends App implements IBootstrap { // File request auth $context->registerEventListener(BeforeTemplateRenderedEvent::class, LoadPublicFileRequestAuthListener::class); + // Update mounts + $context->registerEventListener(ShareCreatedEvent::class, SharesUpdatedListener::class); + $context->registerEventListener(BeforeShareDeletedEvent::class, SharesUpdatedListener::class); + $context->registerEventListener(UserAddedEvent::class, SharesUpdatedListener::class); + $context->registerEventListener(UserRemovedEvent::class, SharesUpdatedListener::class); + $context->registerEventListener(UserShareAccessUpdatedEvent::class, SharesUpdatedListener::class); + $context->registerConfigLexicon(ConfigLexicon::class); } diff --git a/apps/files_sharing/lib/Listener/SharesUpdatedListener.php b/apps/files_sharing/lib/Listener/SharesUpdatedListener.php new file mode 100644 index 00000000000..9a7941888d4 --- /dev/null +++ b/apps/files_sharing/lib/Listener/SharesUpdatedListener.php @@ -0,0 +1,66 @@ + + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +namespace OCA\Files_Sharing\Listener; + +use OCA\Files_Sharing\Event\UserShareAccessUpdatedEvent; +use OCA\Files_Sharing\MountProvider; +use OCA\Files_Sharing\ShareTargetValidator; +use OCP\EventDispatcher\Event; +use OCP\EventDispatcher\IEventListener; +use OCP\Files\Config\ICachedMountInfo; +use OCP\Files\Config\IUserMountCache; +use OCP\Group\Events\UserAddedEvent; +use OCP\Group\Events\UserRemovedEvent; +use OCP\IUser; +use OCP\Share\Events\BeforeShareDeletedEvent; +use OCP\Share\Events\ShareCreatedEvent; +use OCP\Share\IManager; + +/** + * Listen to various events that can change what shares a user has access to + * + * @template-implements IEventListener + */ +class SharesUpdatedListener implements IEventListener { + public function __construct( + private readonly IManager $shareManager, + private readonly IUserMountCache $userMountCache, + private readonly MountProvider $shareMountProvider, + private readonly ShareTargetValidator $shareTargetValidator, + ) { + } + + public function handle(Event $event): void { + if ($event instanceof UserAddedEvent || $event instanceof UserRemovedEvent || $event instanceof UserShareAccessUpdatedEvent) { + $this->updateForUser($event->getUser()); + } + if ($event instanceof ShareCreatedEvent || $event instanceof BeforeShareDeletedEvent) { + foreach ($this->shareManager->getUsersForShare($event->getShare()) as $user) { + $this->updateForUser($user); + } + } + } + + private function updateForUser(IUser $user): void { + $cachedMounts = $this->userMountCache->getMountsForUser($user); + $mountPoints = array_map(fn (ICachedMountInfo $mount) => $mount->getMountPoint(), $cachedMounts); + $mountsByPath = array_combine($mountPoints, $cachedMounts); + + $shares = $this->shareMountProvider->getSuperSharesForUser($user); + + foreach ($shares as &$share) { + [$parentShare, $groupedShares] = $share; + $mountPoint = '/' . $user->getUID() . '/files/' . trim($parentShare->getTarget(), '/') . '/'; + $mountKey = $parentShare->getNodeId() . '::' . $mountPoint; + if (!isset($cachedMounts[$mountKey])) { + $this->shareTargetValidator->verifyMountPoint($user, $parentShare, $mountsByPath, $groupedShares); + } + } + } +} diff --git a/apps/files_sharing/lib/MountProvider.php b/apps/files_sharing/lib/MountProvider.php index 005bf9caf42..790ea11db29 100644 --- a/apps/files_sharing/lib/MountProvider.php +++ b/apps/files_sharing/lib/MountProvider.php @@ -53,6 +53,14 @@ class MountProvider implements IMountProvider, IPartialMountProvider { * @return IMountPoint[] */ public function getMountsForUser(IUser $user, IStorageFactory $loader) { + return array_values($this->getMountsFromSuperShares($user, $this->getSuperSharesForUser($user), $loader)); + } + + /** + * @param IUser $user + * @return list}> Tuple of [superShare, groupedShares] + */ + public function getSuperSharesForUser(IUser $user): array { $userId = $user->getUID(); $shares = $this->mergeIterables( $this->shareManager->getSharedWith($userId, IShare::TYPE_USER, null, -1), @@ -63,16 +71,7 @@ class MountProvider implements IMountProvider, IPartialMountProvider { ); $shares = $this->filterShares($shares, $userId); - $superShares = $this->buildSuperShares($shares, $user); - - return array_values( - $this->getMountsFromSuperShares( - $userId, - $superShares, - $loader, - $user, - ), - ); + return $this->buildSuperShares($shares, $user); } /** @@ -254,18 +253,18 @@ class MountProvider implements IMountProvider, IPartialMountProvider { } /** * @param string $userId - * @param array $superShares + * @param list}> $superShares * @param IStorageFactory $loader * @param IUser $user * @return array IMountPoint indexed by mount point * @throws Exception */ - private function getMountsFromSuperShares( - string $userId, + public function getMountsFromSuperShares( + IUser $user, array $superShares, IStorageFactory $loader, - IUser $user, ): array { + $userId = $user->getUID(); $allMounts = $this->mountManager->getAll(); $mounts = []; $view = new View('/' . $userId . '/files'); @@ -312,7 +311,6 @@ class MountProvider implements IMountProvider, IPartialMountProvider { 'sharingDisabledForUser' => $sharingDisabledForUser ], $loader, - $view, $this->eventDispatcher, $user, ); @@ -399,7 +397,7 @@ class MountProvider implements IMountProvider, IPartialMountProvider { $shares = $this->filterShares($shares, $userId); $superShares = $this->buildSuperShares($shares, $user); - return $this->getMountsFromSuperShares($userId, $superShares, $loader, $user); + return $this->getMountsFromSuperShares($user, $superShares, $loader); } /** diff --git a/apps/files_sharing/lib/ShareTargetValidator.php b/apps/files_sharing/lib/ShareTargetValidator.php index 3dbd3877819..7534cabc33a 100644 --- a/apps/files_sharing/lib/ShareTargetValidator.php +++ b/apps/files_sharing/lib/ShareTargetValidator.php @@ -13,6 +13,7 @@ use OC\Files\SetupManager; use OC\Files\View; use OCP\Cache\CappedMemoryCache; use OCP\EventDispatcher\IEventDispatcher; +use OCP\Files\Config\ICachedMountInfo; use OCP\Files\Mount\IMountManager; use OCP\Files\Mount\IMountPoint; use OCP\IUser; @@ -46,7 +47,7 @@ class ShareTargetValidator { /** * check if the parent folder exists otherwise move the mount point up * - * @param array $allCachedMounts Other mounts for the user, indexed by path + * @param array $allCachedMounts Other mounts for the user, indexed by path * @param IShare[] $childShares * @return string */ @@ -105,7 +106,7 @@ class ShareTargetValidator { /** - * @param IMountPoint[] $allCachedMounts + * @param ICachedMountInfo[] $allCachedMounts */ public function generateUniqueTarget( int $shareNodeId, @@ -131,7 +132,7 @@ class ShareTargetValidator { } /** - * @param IMountPoint[] $allCachedMounts + * @param ICachedMountInfo[] $allCachedMounts */ private function hasConflictingMount(int $shareNodeId, array $allCachedMounts, string $absolutePath): bool { if (!isset($allCachedMounts[$absolutePath . '/'])) { @@ -139,7 +140,7 @@ class ShareTargetValidator { } $mount = $allCachedMounts[$absolutePath . '/']; - if ($mount instanceof SharedMount && $mount->getShare()->getNodeId() === $shareNodeId) { + if ($mount->getMountProvider() === MountProvider::class && $mount->getRootId() === $shareNodeId) { // "conflicting" mount is a mount for the current share return false; } diff --git a/apps/files_sharing/lib/SharedMount.php b/apps/files_sharing/lib/SharedMount.php index 4dff3bcf3b7..8759272eb60 100644 --- a/apps/files_sharing/lib/SharedMount.php +++ b/apps/files_sharing/lib/SharedMount.php @@ -11,7 +11,6 @@ namespace OCA\Files_Sharing; use OC\Files\Filesystem; use OC\Files\Mount\MountPoint; use OC\Files\Mount\MoveableMount; -use OC\Files\View; use OCA\Files_Sharing\Exceptions\BrokenPath; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Events\InvalidateMountCacheEvent; @@ -41,7 +40,6 @@ class SharedMount extends MountPoint implements MoveableMount, ISharedMountPoint $storage, $arguments, IStorageFactory $loader, - private View $recipientView, private IEventDispatcher $eventDispatcher, private IUser $user, ) { @@ -188,4 +186,8 @@ class SharedMount extends MountPoint implements MoveableMount, ISharedMountPoint public function getMountType() { return 'shared'; } + + public function getUser(): IUser { + return $this->user; + } } diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php index 40f768ffab4..e0340b3ce00 100644 --- a/apps/files_sharing/tests/ApiTest.php +++ b/apps/files_sharing/tests/ApiTest.php @@ -826,6 +826,8 @@ class ApiTest extends TestCase { $share3->setStatus(IShare::STATUS_ACCEPTED); $this->shareManager->updateShare($share3); + $this->logout(); + // $request = $this->createRequest(['path' => $this->subfolder]); $ocs = $this->createOCS(self::TEST_FILES_SHARING_API_USER2); $result1 = $ocs->getShares('false', 'false', 'false', $this->subfolder); diff --git a/apps/files_sharing/tests/ShareTargetValidatorTest.php b/apps/files_sharing/tests/ShareTargetValidatorTest.php new file mode 100644 index 00000000000..18fcd35ed28 --- /dev/null +++ b/apps/files_sharing/tests/ShareTargetValidatorTest.php @@ -0,0 +1,141 @@ + + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +namespace OCA\Files_Sharing\Tests; + +use OCA\Files_Sharing\ShareTargetValidator; +use OCP\Constants; +use OCP\Files\Config\ICachedMountInfo; +use OCP\Files\Folder; +use OCP\IUser; +use OCP\Server; +use OCP\Share\IShare; + +#[\PHPUnit\Framework\Attributes\Group('DB')] +class ShareTargetValidatorTest extends TestCase { + private ShareTargetValidator $targetValidator; + + private IUser $user2; + protected string $folder2; + + protected function setUp(): void { + parent::setUp(); + + $this->folder = '/folder_share_storage_test'; + $this->folder2 = '/folder_share_storage_test2'; + + $this->filename = '/share-api-storage.txt'; + + + $this->view->mkdir($this->folder); + $this->view->mkdir($this->folder2); + + // save file with content + $this->view->file_put_contents($this->filename, 'root file'); + $this->view->file_put_contents($this->folder . $this->filename, 'file in subfolder'); + $this->view->file_put_contents($this->folder2 . $this->filename, 'file in subfolder2'); + + $this->targetValidator = Server::get(ShareTargetValidator::class); + $this->user2 = $this->createMock(IUser::class); + $this->user2->method('getUID') + ->willReturn(self::TEST_FILES_SHARING_API_USER2); + } + + + /** + * test if the mount point moves up if the parent folder no longer exists + */ + public function testShareMountLoseParentFolder(): void { + // share to user + $share = $this->share( + IShare::TYPE_USER, + $this->folder, + self::TEST_FILES_SHARING_API_USER1, + self::TEST_FILES_SHARING_API_USER2, + Constants::PERMISSION_ALL); + $this->shareManager->acceptShare($share, self::TEST_FILES_SHARING_API_USER2); + + $share->setTarget('/foo/bar' . $this->folder); + $this->shareManager->moveShare($share, self::TEST_FILES_SHARING_API_USER2); + + $share = $this->shareManager->getShareById($share->getFullId()); + $this->assertSame('/foo/bar' . $this->folder, $share->getTarget()); + + $this->targetValidator->verifyMountPoint($this->user2, $share, [], [$share]); + + $share = $this->shareManager->getShareById($share->getFullId()); + $this->assertSame($this->folder, $share->getTarget()); + + //cleanup + self::loginHelper(self::TEST_FILES_SHARING_API_USER1); + $this->shareManager->deleteShare($share); + $this->view->unlink($this->folder); + } + + /** + * test if the mount point gets renamed if a folder exists at the target + */ + public function testShareMountOverFolder(): void { + self::loginHelper(self::TEST_FILES_SHARING_API_USER2); + $this->view2->mkdir('bar'); + + self::loginHelper(self::TEST_FILES_SHARING_API_USER1); + + // share to user + $share = $this->share( + IShare::TYPE_USER, + $this->folder, + self::TEST_FILES_SHARING_API_USER1, + self::TEST_FILES_SHARING_API_USER2, + Constants::PERMISSION_ALL); + $this->shareManager->acceptShare($share, self::TEST_FILES_SHARING_API_USER2); + + $share->setTarget('/bar'); + $this->shareManager->moveShare($share, self::TEST_FILES_SHARING_API_USER2); + + $share = $this->shareManager->getShareById($share->getFullId()); + + $this->targetValidator->verifyMountPoint($this->user2, $share, [], [$share]); + + $share = $this->shareManager->getShareById($share->getFullId()); + $this->assertSame('/bar (2)', $share->getTarget()); + + //cleanup + self::loginHelper(self::TEST_FILES_SHARING_API_USER1); + $this->shareManager->deleteShare($share); + $this->view->unlink($this->folder); + } + + /** + * test if the mount point gets renamed if another share exists at the target + */ + public function testShareMountOverShare(): void { + // share to user + $share2 = $this->share( + IShare::TYPE_USER, + $this->folder2, + self::TEST_FILES_SHARING_API_USER1, + self::TEST_FILES_SHARING_API_USER2, + Constants::PERMISSION_ALL); + $this->shareManager->acceptShare($share2, self::TEST_FILES_SHARING_API_USER2); + + $conflictingMount = $this->createMock(ICachedMountInfo::class); + $this->targetValidator->verifyMountPoint($this->user2, $share2, [ + '/' . $this->user2->getUID() . '/files' . $this->folder2 . '/' => $conflictingMount + ], [$share2]); + + $share2 = $this->shareManager->getShareById($share2->getFullId()); + + $this->assertSame("{$this->folder2} (2)", $share2->getTarget()); + + //cleanup + self::loginHelper(self::TEST_FILES_SHARING_API_USER1); + $this->shareManager->deleteShare($share2); + $this->view->unlink($this->folder); + } +} diff --git a/apps/files_sharing/tests/SharedMountTest.php b/apps/files_sharing/tests/SharedMountTest.php index 48cd940bf50..020b6ec41c5 100644 --- a/apps/files_sharing/tests/SharedMountTest.php +++ b/apps/files_sharing/tests/SharedMountTest.php @@ -8,12 +8,8 @@ namespace OCA\Files_Sharing\Tests; use OC\Files\Filesystem; -use OC\Files\View; -use OC\Memcache\ArrayCache; -use OCA\Files_Sharing\MountProvider; use OCA\Files_Sharing\SharedMount; use OCP\Constants; -use OCP\ICacheFactory; use OCP\IDBConnection; use OCP\IGroupManager; use OCP\IUserManager; @@ -25,14 +21,10 @@ use OCP\Share\IShare; */ #[\PHPUnit\Framework\Attributes\Group(name: 'SLOWDB')] class SharedMountTest extends TestCase { + private IGroupManager $groupManager; + private IUserManager $userManager; - /** @var IGroupManager */ - private $groupManager; - - /** @var IUserManager */ - private $userManager; - - private $folder2; + private string $folder2; protected function setUp(): void { parent::setUp(); @@ -68,78 +60,6 @@ class SharedMountTest extends TestCase { parent::tearDown(); } - /** - * test if the mount point moves up if the parent folder no longer exists - */ - public function testShareMountLoseParentFolder(): void { - - // share to user - $share = $this->share( - IShare::TYPE_USER, - $this->folder, - self::TEST_FILES_SHARING_API_USER1, - self::TEST_FILES_SHARING_API_USER2, - Constants::PERMISSION_ALL); - $this->shareManager->acceptShare($share, self::TEST_FILES_SHARING_API_USER2); - - $share->setTarget('/foo/bar' . $this->folder); - $this->shareManager->moveShare($share, self::TEST_FILES_SHARING_API_USER2); - - $share = $this->shareManager->getShareById($share->getFullId()); - $this->assertSame('/foo/bar' . $this->folder, $share->getTarget()); - - self::loginHelper(self::TEST_FILES_SHARING_API_USER2); - // share should have moved up - - $share = $this->shareManager->getShareById($share->getFullId()); - $this->assertSame($this->folder, $share->getTarget()); - - //cleanup - self::loginHelper(self::TEST_FILES_SHARING_API_USER1); - $this->shareManager->deleteShare($share); - $this->view->unlink($this->folder); - } - - public function testDeleteParentOfMountPoint(): void { - // share to user - $share = $this->share( - IShare::TYPE_USER, - $this->folder, - self::TEST_FILES_SHARING_API_USER1, - self::TEST_FILES_SHARING_API_USER2, - Constants::PERMISSION_ALL - ); - - self::loginHelper(self::TEST_FILES_SHARING_API_USER2); - $user2View = new View('/' . self::TEST_FILES_SHARING_API_USER2 . '/files'); - $this->assertTrue($user2View->file_exists($this->folder)); - - // create a local folder - $result = $user2View->mkdir('localfolder'); - $this->assertTrue($result); - - // move mount point to local folder - $result = $user2View->rename($this->folder, '/localfolder/' . $this->folder); - $this->assertTrue($result); - - // mount point in the root folder should no longer exist - $this->assertFalse($user2View->is_dir($this->folder)); - - // delete the local folder - $result = $user2View->unlink('/localfolder'); - $this->assertTrue($result); - - //enforce reload of the mount points - self::loginHelper(self::TEST_FILES_SHARING_API_USER2); - - //mount point should be back at the root - $this->assertTrue($user2View->is_dir($this->folder)); - - //cleanup - self::loginHelper(self::TEST_FILES_SHARING_API_USER1); - $this->view->unlink($this->folder); - } - public function testMoveSharedFile(): void { $share = $this->share( IShare::TYPE_USER, @@ -313,111 +233,6 @@ class SharedMountTest extends TestCase { $testGroup->removeUser($user2); $testGroup->removeUser($user3); } - - /** - * test if the mount point gets renamed if a folder exists at the target - */ - public function testShareMountOverFolder(): void { - self::loginHelper(self::TEST_FILES_SHARING_API_USER2); - $this->view2->mkdir('bar'); - - self::loginHelper(self::TEST_FILES_SHARING_API_USER1); - - // share to user - $share = $this->share( - IShare::TYPE_USER, - $this->folder, - self::TEST_FILES_SHARING_API_USER1, - self::TEST_FILES_SHARING_API_USER2, - Constants::PERMISSION_ALL); - $this->shareManager->acceptShare($share, self::TEST_FILES_SHARING_API_USER2); - - $share->setTarget('/bar'); - $this->shareManager->moveShare($share, self::TEST_FILES_SHARING_API_USER2); - - $share = $this->shareManager->getShareById($share->getFullId()); - - self::loginHelper(self::TEST_FILES_SHARING_API_USER2); - // share should have been moved - - $share = $this->shareManager->getShareById($share->getFullId()); - $this->assertSame('/bar (2)', $share->getTarget()); - - //cleanup - self::loginHelper(self::TEST_FILES_SHARING_API_USER1); - $this->shareManager->deleteShare($share); - $this->view->unlink($this->folder); - } - - /** - * test if the mount point gets renamed if another share exists at the target - */ - public function testShareMountOverShare(): void { - // create a shared cache - $caches = []; - $cacheFactory = $this->createMock(ICacheFactory::class); - $cacheFactory->method('createLocal') - ->willReturnCallback(function (string $prefix) use (&$caches) { - if (!isset($caches[$prefix])) { - $caches[$prefix] = new ArrayCache($prefix); - } - return $caches[$prefix]; - }); - $cacheFactory->method('createDistributed') - ->willReturnCallback(function (string $prefix) use (&$caches) { - if (!isset($caches[$prefix])) { - $caches[$prefix] = new ArrayCache($prefix); - } - return $caches[$prefix]; - }); - - // hack to overwrite the cache factory, we can't use the proper "overwriteService" since the mount provider is created before this test is called - $mountProvider = Server::get(MountProvider::class); - $reflectionClass = new \ReflectionClass($mountProvider); - $reflectionCacheFactory = $reflectionClass->getProperty('cacheFactory'); - $reflectionCacheFactory->setValue($mountProvider, $cacheFactory); - - // share to user - $share = $this->share( - IShare::TYPE_USER, - $this->folder, - self::TEST_FILES_SHARING_API_USER1, - self::TEST_FILES_SHARING_API_USER2, - Constants::PERMISSION_ALL); - $this->shareManager->acceptShare($share, self::TEST_FILES_SHARING_API_USER2); - - $share->setTarget('/foobar'); - $this->shareManager->moveShare($share, self::TEST_FILES_SHARING_API_USER2); - - - // share to user - $share2 = $this->share( - IShare::TYPE_USER, - $this->folder2, - self::TEST_FILES_SHARING_API_USER1, - self::TEST_FILES_SHARING_API_USER2, - Constants::PERMISSION_ALL); - $this->shareManager->acceptShare($share2, self::TEST_FILES_SHARING_API_USER2); - - $share2->setTarget('/foobar'); - $this->shareManager->moveShare($share2, self::TEST_FILES_SHARING_API_USER2); - - self::loginHelper(self::TEST_FILES_SHARING_API_USER2); - // one of the shares should have been moved - - $share = $this->shareManager->getShareById($share->getFullId()); - $share2 = $this->shareManager->getShareById($share2->getFullId()); - - // we don't know or care which share got the "(2)" just that one of them did - $this->assertNotEquals($share->getTarget(), $share2->getTarget()); - $this->assertSame('/foobar', min($share->getTarget(), $share2->getTarget())); - $this->assertSame('/foobar (2)', max($share->getTarget(), $share2->getTarget())); - - //cleanup - self::loginHelper(self::TEST_FILES_SHARING_API_USER1); - $this->shareManager->deleteShare($share); - $this->view->unlink($this->folder); - } } class DummyTestClassSharedMount extends SharedMount { diff --git a/apps/files_sharing/tests/SharedStorageTest.php b/apps/files_sharing/tests/SharedStorageTest.php index ada3d84734c..cd0a06f2436 100644 --- a/apps/files_sharing/tests/SharedStorageTest.php +++ b/apps/files_sharing/tests/SharedStorageTest.php @@ -10,7 +10,6 @@ namespace OCA\Files_Sharing\Tests; use OC\Files\Cache\FailedCache; use OC\Files\Filesystem; use OC\Files\Storage\FailedStorage; -use OC\Files\Storage\Storage; use OC\Files\Storage\Temporary; use OC\Files\View; use OCA\Files_Sharing\SharedStorage; @@ -60,51 +59,6 @@ class SharedStorageTest extends TestCase { parent::tearDown(); } - /** - * if the parent of the mount point is gone then the mount point should move up - */ - public function testParentOfMountPointIsGone(): void { - - // share to user - $share = $this->share( - IShare::TYPE_USER, - $this->folder, - self::TEST_FILES_SHARING_API_USER1, - self::TEST_FILES_SHARING_API_USER2, - Constants::PERMISSION_ALL - ); - - self::loginHelper(self::TEST_FILES_SHARING_API_USER2); - $user2View = new View('/' . self::TEST_FILES_SHARING_API_USER2 . '/files'); - $this->assertTrue($user2View->file_exists($this->folder)); - - // create a local folder - $result = $user2View->mkdir('localfolder'); - $this->assertTrue($result); - - // move mount point to local folder - $result = $user2View->rename($this->folder, '/localfolder/' . $this->folder); - $this->assertTrue($result); - - // mount point in the root folder should no longer exist - $this->assertFalse($user2View->is_dir($this->folder)); - - // delete the local folder - /** @var Storage $storage */ - [$storage, $internalPath] = Filesystem::resolvePath('/' . self::TEST_FILES_SHARING_API_USER2 . '/files/localfolder'); - $storage->rmdir($internalPath); - - //enforce reload of the mount points - self::loginHelper(self::TEST_FILES_SHARING_API_USER2); - - //mount point should be back at the root - $this->assertTrue($user2View->is_dir($this->folder)); - - //cleanup - self::loginHelper(self::TEST_FILES_SHARING_API_USER1); - $this->view->unlink($this->folder); - } - public function testRenamePartFile(): void { // share to user @@ -466,57 +420,6 @@ class SharedStorageTest extends TestCase { $this->shareManager->deleteShare($share); } - public function testNameConflict(): void { - self::loginHelper(self::TEST_FILES_SHARING_API_USER1); - $view1 = new View('/' . self::TEST_FILES_SHARING_API_USER1 . '/files'); - $view1->mkdir('foo'); - - self::loginHelper(self::TEST_FILES_SHARING_API_USER3); - $view3 = new View('/' . self::TEST_FILES_SHARING_API_USER3 . '/files'); - $view3->mkdir('foo'); - - // share a folder with the same name from two different users to the same user - self::loginHelper(self::TEST_FILES_SHARING_API_USER1); - - $share1 = $this->share( - IShare::TYPE_GROUP, - 'foo', - self::TEST_FILES_SHARING_API_USER1, - self::TEST_FILES_SHARING_API_GROUP1, - Constants::PERMISSION_ALL - ); - $this->shareManager->acceptShare($share1, self::TEST_FILES_SHARING_API_USER2); - - self::loginHelper(self::TEST_FILES_SHARING_API_USER2); - - self::loginHelper(self::TEST_FILES_SHARING_API_USER3); - - $share2 = $this->share( - IShare::TYPE_GROUP, - 'foo', - self::TEST_FILES_SHARING_API_USER3, - self::TEST_FILES_SHARING_API_GROUP1, - Constants::PERMISSION_ALL - ); - $this->shareManager->acceptShare($share2, self::TEST_FILES_SHARING_API_USER2); - - self::loginHelper(self::TEST_FILES_SHARING_API_USER2); - $view2 = new View('/' . self::TEST_FILES_SHARING_API_USER2 . '/files'); - - $this->assertTrue($view2->file_exists('/foo')); - $this->assertTrue($view2->file_exists('/foo (2)')); - - $mount = $view2->getMount('/foo'); - $this->assertInstanceOf('\OCA\Files_Sharing\SharedMount', $mount); - /** @var SharedStorage $storage */ - $storage = $mount->getStorage(); - - $this->assertEquals(self::TEST_FILES_SHARING_API_USER1, $storage->getOwner('')); - - $this->shareManager->deleteShare($share1); - $this->shareManager->deleteShare($share2); - } - public function testOwnerPermissions(): void { self::loginHelper(self::TEST_FILES_SHARING_API_USER1); diff --git a/apps/files_versions/tests/VersioningTest.php b/apps/files_versions/tests/VersioningTest.php index 6faa324f9ca..fdc37016b55 100644 --- a/apps/files_versions/tests/VersioningTest.php +++ b/apps/files_versions/tests/VersioningTest.php @@ -104,6 +104,8 @@ class VersioningTest extends \Test\TestCase { \OC::registerShareHooks(Server::get(SystemConfig::class)); \OC::$server->boot(); + // ensure both users have an up-to-date state + self::loginHelper(self::TEST_VERSIONS_USER2); self::loginHelper(self::TEST_VERSIONS_USER); $this->rootView = new View(); if (!$this->rootView->file_exists(self::USERS_VERSIONS_ROOT)) { diff --git a/build/integration/sharing_features/sharing-v1-part2.feature b/build/integration/sharing_features/sharing-v1-part2.feature index a6e4c67165a..0c83975fc39 100644 --- a/build/integration/sharing_features/sharing-v1-part2.feature +++ b/build/integration/sharing_features/sharing-v1-part2.feature @@ -43,7 +43,7 @@ Feature: sharing | item_type | file | | mimetype | text/plain | | storage_id | shared::/textfile0 (2).txt | - | file_target | /textfile0.txt | + | file_target | /textfile0 (2).txt | | share_with | user2 | | share_with_displayname | user2 | @@ -84,7 +84,7 @@ Feature: sharing | item_type | file | | mimetype | text/plain | | storage_id | shared::/textfile0 (2).txt | - | file_target | /textfile0.txt | + | file_target | /textfile0 (2).txt | | share_with | user2 | | share_with_displayname | user2 | @@ -120,7 +120,7 @@ Feature: sharing | share_type | 0 | | share_with | user1 | | file_source | A_NUMBER | - | file_target | /textfile0.txt | + | file_target | /textfile0 (2).txt | | path | /textfile0.txt | | permissions | 19 | | stime | A_NUMBER | @@ -401,7 +401,7 @@ Feature: sharing | item_type | file | | mimetype | text/plain | | storage_id | shared::/FOLDER/textfile0.txt | - | file_target | /textfile0.txt | + | file_target | /textfile0 (2).txt | | share_with | user2 | | share_with_displayname | user2 | @@ -440,7 +440,7 @@ Feature: sharing | item_type | file | | mimetype | text/plain | | storage_id | shared::/FOLDER/textfile0 (2).txt | - | file_target | /textfile0.txt | + | file_target | /textfile0 (2).txt | | share_with | user2 | | share_with_displayname | user2 | @@ -887,7 +887,7 @@ Feature: sharing | share_type | 0 | | share_with | user2 | | file_source | A_NUMBER | - | file_target | /textfile0.txt | + | file_target | /textfile0 (2).txt | | path | /textfile0 (2).txt | | permissions | 19 | | stime | A_NUMBER | diff --git a/build/integration/sharing_features/sharing-v1.feature b/build/integration/sharing_features/sharing-v1.feature index 25f168db2e7..dad3d6ee6fd 100644 --- a/build/integration/sharing_features/sharing-v1.feature +++ b/build/integration/sharing_features/sharing-v1.feature @@ -559,6 +559,8 @@ Feature: sharing Scenario: getting all shares of a user using that user Given user "user0" exists And user "user1" exists + When User "user1" deletes file "/textfile0.txt" + And the HTTP status code should be "204" And file "textfile0.txt" of user "user0" is shared with user "user1" And As an "user0" When sending "GET" to "/apps/files_sharing/api/v1/shares" diff --git a/lib/private/Files/Config/UserMountCache.php b/lib/private/Files/Config/UserMountCache.php index 3250ba371e8..e41ba2059b8 100644 --- a/lib/private/Files/Config/UserMountCache.php +++ b/lib/private/Files/Config/UserMountCache.php @@ -537,4 +537,13 @@ class UserMountCache implements IUserMountCache { 'mount_provider_class' => $mountProvider ]); } + + /** + * Clear the internal in-memory caches + */ + public function flush(): void { + $this->cacheInfoCache = new CappedMemoryCache(); + $this->internalPathCache = new CappedMemoryCache(); + $this->mountsForUsers = new CappedMemoryCache(); + } } diff --git a/tests/lib/TestCase.php b/tests/lib/TestCase.php index 6fb5bc309ed..551c1024e0b 100644 --- a/tests/lib/TestCase.php +++ b/tests/lib/TestCase.php @@ -13,6 +13,7 @@ use OC\Command\QueueBus; use OC\Files\AppData\Factory; use OC\Files\Cache\Storage; use OC\Files\Config\MountProviderCollection; +use OC\Files\Config\UserMountCache; use OC\Files\Filesystem; use OC\Files\Mount\CacheMountProvider; use OC\Files\Mount\LocalHomeMountProvider; @@ -180,6 +181,8 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase { Storage::getGlobalCache()->clearCache(); } + Server::get(UserMountCache::class)->flush(); + // tearDown the traits $traits = $this->getTestTraits(); foreach ($traits as $trait) {