From 715e7143f042f537ecbc2fcfd936e1996dec1ade Mon Sep 17 00:00:00 2001 From: provokateurin Date: Thu, 14 Nov 2024 07:41:41 +0100 Subject: [PATCH] fix(files_sharing,files): Do not validate shares before adjusting the owner Signed-off-by: provokateurin --- .../lib/Service/OwnershipTransferService.php | 4 ++-- apps/files_sharing/lib/Updater.php | 18 ++++++++------ lib/private/Share20/Manager.php | 24 +++++++++++-------- lib/public/Share/IManager.php | 9 ++++--- 4 files changed, 33 insertions(+), 22 deletions(-) diff --git a/apps/files/lib/Service/OwnershipTransferService.php b/apps/files/lib/Service/OwnershipTransferService.php index 80c61d39ef5b5..71764019abf3e 100644 --- a/apps/files/lib/Service/OwnershipTransferService.php +++ b/apps/files/lib/Service/OwnershipTransferService.php @@ -310,7 +310,7 @@ private function collectUsersShares( foreach ($supportedShareTypes as $shareType) { $offset = 0; while (true) { - $sharePage = $this->shareManager->getSharesBy($sourceUid, $shareType, null, true, 50, $offset); + $sharePage = $this->shareManager->getSharesBy($sourceUid, $shareType, null, true, 50, $offset, onlyValid: false); $progress->advance(count($sharePage)); if (empty($sharePage)) { break; @@ -464,7 +464,7 @@ private function restoreShares( } $share->setNodeId($newNodeId); - $this->shareManager->updateShare($share); + $this->shareManager->updateShare($share, onlyValid: false); } } } catch (NotFoundException $e) { diff --git a/apps/files_sharing/lib/Updater.php b/apps/files_sharing/lib/Updater.php index 82075fb9ba5ca..226cf3e7fd489 100644 --- a/apps/files_sharing/lib/Updater.php +++ b/apps/files_sharing/lib/Updater.php @@ -50,10 +50,14 @@ private static function moveShareInOrOutOfShare($path): void { $shareManager = \OC::$server->getShareManager(); + // We intentionally include invalid shares, as they have been automatically invalidated due to the node no longer + // being accessible for the user. Only in this case where we adjust the share after it was moved we want to ignore + // this to be able to still adjust it. + // FIXME: should CIRCLES be included here ?? - $shares = $shareManager->getSharesBy($user->getUID(), IShare::TYPE_USER, $src, false, -1); - $shares = array_merge($shares, $shareManager->getSharesBy($user->getUID(), IShare::TYPE_GROUP, $src, false, -1)); - $shares = array_merge($shares, $shareManager->getSharesBy($user->getUID(), IShare::TYPE_ROOM, $src, false, -1)); + $shares = $shareManager->getSharesBy($user->getUID(), IShare::TYPE_USER, $src, false, -1, onlyValid: false); + $shares = array_merge($shares, $shareManager->getSharesBy($user->getUID(), IShare::TYPE_GROUP, $src, false, -1, onlyValid: false)); + $shares = array_merge($shares, $shareManager->getSharesBy($user->getUID(), IShare::TYPE_ROOM, $src, false, -1, onlyValid: false)); if ($src instanceof Folder) { $cacheAccess = Server::get(FileAccess::class); @@ -61,9 +65,9 @@ private static function moveShareInOrOutOfShare($path): void { $sourceStorageId = $src->getStorage()->getCache()->getNumericStorageId(); $sourceInternalPath = $src->getInternalPath(); $subShares = array_merge( - $shareManager->getSharesBy($user->getUID(), IShare::TYPE_USER), - $shareManager->getSharesBy($user->getUID(), IShare::TYPE_GROUP), - $shareManager->getSharesBy($user->getUID(), IShare::TYPE_ROOM), + $shareManager->getSharesBy($user->getUID(), IShare::TYPE_USER, onlyValid: false), + $shareManager->getSharesBy($user->getUID(), IShare::TYPE_GROUP, onlyValid: false), + $shareManager->getSharesBy($user->getUID(), IShare::TYPE_ROOM, onlyValid: false), ); $shareSourceIds = array_map(fn (IShare $share) => $share->getNodeId(), $subShares); $shareSources = $cacheAccess->getByFileIdsInStorage($shareSourceIds, $sourceStorageId); @@ -111,7 +115,7 @@ private static function moveShareInOrOutOfShare($path): void { $share->setShareOwner($newOwner); $share->setPermissions($newPermissions); - $shareManager->updateShare($share); + $shareManager->updateShare($share, onlyValid: false); } } diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 6348292088af4..4dadcdbfcd7f4 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -786,13 +786,13 @@ public function createShare(IShare $share) { * @throws \InvalidArgumentException * @throws GenericShareException */ - public function updateShare(IShare $share) { + public function updateShare(IShare $share, bool $onlyValid = true) { $expirationDateUpdated = false; $this->canShare($share); try { - $originalShare = $this->getShareById($share->getFullId()); + $originalShare = $this->getShareById($share->getFullId(), onlyValid: $onlyValid); } catch (\UnexpectedValueException $e) { throw new \InvalidArgumentException($this->l->t('Share does not have a full ID')); } @@ -1241,7 +1241,7 @@ public function getSharesInFolder($userId, Folder $node, $reshares = false, $sha /** * @inheritdoc */ - public function getSharesBy($userId, $shareType, $path = null, $reshares = false, $limit = 50, $offset = 0) { + public function getSharesBy($userId, $shareType, $path = null, $reshares = false, $limit = 50, $offset = 0, bool $onlyValid = true) { if ($path !== null && !($path instanceof \OCP\Files\File) && !($path instanceof \OCP\Files\Folder)) { @@ -1270,11 +1270,13 @@ public function getSharesBy($userId, $shareType, $path = null, $reshares = false while (true) { $added = 0; foreach ($shares as $share) { - try { - $this->checkShare($share); - } catch (ShareNotFound $e) { - // Ignore since this basically means the share is deleted - continue; + if ($onlyValid) { + try { + $this->checkShare($share); + } catch (ShareNotFound $e) { + // Ignore since this basically means the share is deleted + continue; + } } $added++; @@ -1366,7 +1368,7 @@ public function getDeletedSharedWith($userId, $shareType, $node = null, $limit = /** * @inheritdoc */ - public function getShareById($id, $recipient = null) { + public function getShareById($id, $recipient = null, bool $onlyValid = true) { if ($id === null) { throw new ShareNotFound(); } @@ -1381,7 +1383,9 @@ public function getShareById($id, $recipient = null) { $share = $provider->getShareById($id, $recipient); - $this->checkShare($share); + if ($onlyValid) { + $this->checkShare($share); + } return $share; } diff --git a/lib/public/Share/IManager.php b/lib/public/Share/IManager.php index b07bc8f80515e..c3a8494a5ac6b 100644 --- a/lib/public/Share/IManager.php +++ b/lib/public/Share/IManager.php @@ -41,11 +41,12 @@ public function createShare(IShare $share); * The state can't be changed this way: use acceptShare * * @param IShare $share + * @param bool $onlyValid Only updates valid shares, invalid shares will be deleted automatically and are not updated * @return IShare The share object * @throws \InvalidArgumentException * @since 9.0.0 */ - public function updateShare(IShare $share); + public function updateShare(IShare $share, bool $onlyValid = true); /** * Accept a share. @@ -127,10 +128,11 @@ public function getSharesInFolder($userId, Folder $node, $reshares = false, $sha * @param bool $reshares * @param int $limit The maximum number of returned results, -1 for all results * @param int $offset + * @param bool $onlyValid Only returns valid shares, invalid shares will be deleted automatically and are not returned * @return IShare[] * @since 9.0.0 */ - public function getSharesBy($userId, $shareType, $path = null, $reshares = false, $limit = 50, $offset = 0); + public function getSharesBy($userId, $shareType, $path = null, $reshares = false, $limit = 50, $offset = 0, bool $onlyValid = true); /** * Get shares shared with $user. @@ -168,11 +170,12 @@ public function getDeletedSharedWith($userId, $shareType, $node = null, $limit = * * @param string $id * @param string|null $recipient userID of the recipient + * @param bool $onlyValid Only returns valid shares, invalid shares will be deleted automatically and are not returned * @return IShare * @throws ShareNotFound * @since 9.0.0 */ - public function getShareById($id, $recipient = null); + public function getShareById($id, $recipient = null, bool $onlyValid = true); /** * Get the share by token possible with password