From 2fef6c2e6cf00a6a2bd84497b6fd210598de9f69 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 12 Feb 2026 15:34:50 +0100 Subject: [PATCH] chore: move share recipient validation logic to a separate class Signed-off-by: Robin Appelman --- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + .../lib/Listener/SharesUpdatedListener.php | 73 ++-------------- .../lib/ShareRecipientUpdater.php | 86 +++++++++++++++++++ 4 files changed, 94 insertions(+), 67 deletions(-) create mode 100644 apps/files_sharing/lib/ShareRecipientUpdater.php diff --git a/apps/files_sharing/composer/composer/autoload_classmap.php b/apps/files_sharing/composer/composer/autoload_classmap.php index 0ba6ba1b816..c3dea9de9ce 100644 --- a/apps/files_sharing/composer/composer/autoload_classmap.php +++ b/apps/files_sharing/composer/composer/autoload_classmap.php @@ -98,6 +98,7 @@ return array( 'OCA\\Files_Sharing\\Settings\\Personal' => $baseDir . '/../lib/Settings/Personal.php', 'OCA\\Files_Sharing\\ShareBackend\\File' => $baseDir . '/../lib/ShareBackend/File.php', 'OCA\\Files_Sharing\\ShareBackend\\Folder' => $baseDir . '/../lib/ShareBackend/Folder.php', + 'OCA\\Files_Sharing\\ShareRecipientUpdater' => $baseDir . '/../lib/ShareRecipientUpdater.php', 'OCA\\Files_Sharing\\ShareTargetValidator' => $baseDir . '/../lib/ShareTargetValidator.php', 'OCA\\Files_Sharing\\SharedMount' => $baseDir . '/../lib/SharedMount.php', 'OCA\\Files_Sharing\\SharedStorage' => $baseDir . '/../lib/SharedStorage.php', diff --git a/apps/files_sharing/composer/composer/autoload_static.php b/apps/files_sharing/composer/composer/autoload_static.php index 03906cda047..57cf4cc29fb 100644 --- a/apps/files_sharing/composer/composer/autoload_static.php +++ b/apps/files_sharing/composer/composer/autoload_static.php @@ -113,6 +113,7 @@ class ComposerStaticInitFiles_Sharing 'OCA\\Files_Sharing\\Settings\\Personal' => __DIR__ . '/..' . '/../lib/Settings/Personal.php', 'OCA\\Files_Sharing\\ShareBackend\\File' => __DIR__ . '/..' . '/../lib/ShareBackend/File.php', 'OCA\\Files_Sharing\\ShareBackend\\Folder' => __DIR__ . '/..' . '/../lib/ShareBackend/Folder.php', + 'OCA\\Files_Sharing\\ShareRecipientUpdater' => __DIR__ . '/..' . '/../lib/ShareRecipientUpdater.php', 'OCA\\Files_Sharing\\ShareTargetValidator' => __DIR__ . '/..' . '/../lib/ShareTargetValidator.php', 'OCA\\Files_Sharing\\SharedMount' => __DIR__ . '/..' . '/../lib/SharedMount.php', 'OCA\\Files_Sharing\\SharedStorage' => __DIR__ . '/..' . '/../lib/SharedStorage.php', diff --git a/apps/files_sharing/lib/Listener/SharesUpdatedListener.php b/apps/files_sharing/lib/Listener/SharesUpdatedListener.php index fea681bd31c..3cfe3c3a3a5 100644 --- a/apps/files_sharing/lib/Listener/SharesUpdatedListener.php +++ b/apps/files_sharing/lib/Listener/SharesUpdatedListener.php @@ -8,23 +8,16 @@ declare(strict_types=1); namespace OCA\Files_Sharing\Listener; -use OC\Files\FileInfo; use OCA\Files_Sharing\Event\UserShareAccessUpdatedEvent; -use OCA\Files_Sharing\MountProvider; -use OCA\Files_Sharing\ShareTargetValidator; +use OCA\Files_Sharing\ShareRecipientUpdater; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; -use OCP\Files\Config\ICachedMountInfo; -use OCP\Files\Config\IUserMountCache; -use OCP\Files\Storage\IStorageFactory; 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\Events\ShareTransferredEvent; use OCP\Share\IManager; -use OCP\Share\IShare; /** * Listen to various events that can change what shares a user has access to @@ -32,31 +25,26 @@ use OCP\Share\IShare; * @template-implements IEventListener */ class SharesUpdatedListener implements IEventListener { - private array $inUpdate = []; - public function __construct( private readonly IManager $shareManager, - private readonly IUserMountCache $userMountCache, - private readonly MountProvider $shareMountProvider, - private readonly ShareTargetValidator $shareTargetValidator, - private readonly IStorageFactory $storageFactory, + private readonly ShareRecipientUpdater $shareUpdater, ) { } public function handle(Event $event): void { if ($event instanceof UserShareAccessUpdatedEvent) { foreach ($event->getUsers() as $user) { - $this->updateForUser($user, true); + $this->shareUpdater->updateForUser($user, true); } } if ($event instanceof UserAddedEvent || $event instanceof UserRemovedEvent) { - $this->updateForUser($event->getUser(), true); + $this->shareUpdater->updateForUser($event->getUser(), true); } if ($event instanceof ShareCreatedEvent || $event instanceof ShareTransferredEvent) { $share = $event->getShare(); $shareTarget = $share->getTarget(); foreach ($this->shareManager->getUsersForShare($share) as $user) { if ($share->getSharedBy() !== $user->getUID()) { - $this->updateForShare($user, $share); + $this->shareUpdater->updateForShare($user, $share); // Share target validation might have changed the target, restore it for the next user $share->setTarget($shareTarget); } @@ -64,57 +52,8 @@ class SharesUpdatedListener implements IEventListener { } if ($event instanceof BeforeShareDeletedEvent) { foreach ($this->shareManager->getUsersForShare($event->getShare()) as $user) { - $this->updateForUser($user, false, [$event->getShare()]); + $this->shareManager->updateForUser($user, false, [$event->getShare()]); } } } - - private function updateForUser(IUser $user, bool $verifyMountPoints, array $ignoreShares = []): void { - // prevent recursion - if (isset($this->inUpdate[$user->getUID()])) { - return; - } - $this->inUpdate[$user->getUID()] = true; - $cachedMounts = $this->userMountCache->getMountsForUser($user); - $shareMounts = array_filter($cachedMounts, fn (ICachedMountInfo $mount) => $mount->getMountProvider() === MountProvider::class); - $mountPoints = array_map(fn (ICachedMountInfo $mount) => $mount->getMountPoint(), $cachedMounts); - $mountsByPath = array_combine($mountPoints, $cachedMounts); - - $shares = $this->shareMountProvider->getSuperSharesForUser($user, $ignoreShares); - - $mountsChanged = count($shares) !== count($shareMounts); - foreach ($shares as &$share) { - [$parentShare, $groupedShares] = $share; - $mountPoint = '/' . $user->getUID() . '/files/' . trim($parentShare->getTarget(), '/') . '/'; - $mountKey = $parentShare->getNodeId() . '::' . $mountPoint; - if (!isset($cachedMounts[$mountKey])) { - $mountsChanged = true; - if ($verifyMountPoints) { - $this->shareTargetValidator->verifyMountPoint($user, $parentShare, $mountsByPath, $groupedShares); - } - } - } - - if ($mountsChanged) { - $newMounts = $this->shareMountProvider->getMountsFromSuperShares($user, $shares, $this->storageFactory); - $this->userMountCache->registerMounts($user, $newMounts, [MountProvider::class]); - } - - unset($this->inUpdate[$user->getUID()]); - } - - private function updateForShare(IUser $user, IShare $share): void { - $cachedMounts = $this->userMountCache->getMountsForUser($user); - $mountPoints = array_map(fn (ICachedMountInfo $mount) => $mount->getMountPoint(), $cachedMounts); - $mountsByPath = array_combine($mountPoints, $cachedMounts); - - $target = $this->shareTargetValidator->verifyMountPoint($user, $share, $mountsByPath, [$share]); - $mountPoint = '/' . $user->getUID() . '/files/' . trim($target, '/') . '/'; - - $fileInfo = $share->getNode(); - if (!$fileInfo instanceof FileInfo) { - throw new \Exception("share node is the wrong fileinfo"); - } - $this->userMountCache->addMount($user, $mountPoint, $fileInfo->getData(), MountProvider::class); - } } diff --git a/apps/files_sharing/lib/ShareRecipientUpdater.php b/apps/files_sharing/lib/ShareRecipientUpdater.php new file mode 100644 index 00000000000..979dc41dfb4 --- /dev/null +++ b/apps/files_sharing/lib/ShareRecipientUpdater.php @@ -0,0 +1,86 @@ +inUpdate[$user->getUID()])) { + return; + } + $this->inUpdate[$user->getUID()] = true; + + $cachedMounts = $this->userMountCache->getMountsForUser($user); + $shareMounts = array_filter($cachedMounts, fn (ICachedMountInfo $mount) => $mount->getMountProvider() === MountProvider::class); + $mountPoints = array_map(fn (ICachedMountInfo $mount) => $mount->getMountPoint(), $cachedMounts); + $mountsByPath = array_combine($mountPoints, $cachedMounts); + + $shares = $this->shareMountProvider->getSuperSharesForUser($user, $ignoreShares); + + // the share mounts have changed if either the number of shares doesn't matched the number of share mounts + // or there is a share for which we don't have a mount yet. + $mountsChanged = count($shares) !== count($shareMounts); + foreach ($shares as &$share) { + [$parentShare, $groupedShares] = $share; + $mountPoint = '/' . $user->getUID() . '/files/' . trim($parentShare->getTarget(), '/') . '/'; + $mountKey = $parentShare->getNodeId() . '::' . $mountPoint; + if (!isset($cachedMounts[$mountKey])) { + $mountsChanged = true; + if ($verifyMountPoints) { + $this->shareTargetValidator->verifyMountPoint($user, $parentShare, $mountsByPath, $groupedShares); + } + } + } + + if ($mountsChanged) { + $newMounts = $this->shareMountProvider->getMountsFromSuperShares($user, $shares, $this->storageFactory); + $this->userMountCache->registerMounts($user, $newMounts, [MountProvider::class]); + } + + unset($this->inUpdate[$user->getUID()]); + } + + /** + * Validate a single received share for a user + */ + public function updateForShare(IUser $user, IShare $share): void { + $cachedMounts = $this->userMountCache->getMountsForUser($user); + $mountPoints = array_map(fn (ICachedMountInfo $mount) => $mount->getMountPoint(), $cachedMounts); + $mountsByPath = array_combine($mountPoints, $cachedMounts); + + $target = $this->shareTargetValidator->verifyMountPoint($user, $share, $mountsByPath, [$share]); + $mountPoint = '/' . $user->getUID() . '/files/' . trim($target, '/') . '/'; + + $fileInfo = $share->getNode(); + if (!$fileInfo instanceof FileInfo) { + throw new \Exception('share node is the wrong fileinfo'); + } + $this->userMountCache->addMount($user, $mountPoint, $fileInfo->getData(), MountProvider::class); + } +}