From 86947cb96e16e372b7821ec30e6ffc2323dc010e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 22 Mar 2024 14:15:57 +0100 Subject: [PATCH] fix: Fix federated group shares when no longer found in remote server MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a federated group share with another server is deleted a notification is sent to the remote server to delete the share. If that notification is not received then when the remote server tries to access the share again it will not be found in the sharer server, so it will be locally unshared in the remote server. This works fine for user shares, as in that case the share is deleted in the remote server, but locally unsharing a received federated group share only marks it as pending again for the user that tried to access it. Due to that the received federated group share is left dangling in the remote server, and it is kept forever in the list of pending shares for the users in the group. To solve that now the received federated group share is fully removed instead of simply unshared when no longer available in the sharer server. Note that all of the above applies only when the sharer server is still available and it returns a "not found" (or "forbidden") error when the remote server tries to access the share. If the sharer server is not available accessing the received share does not fully remove the group share, as in that case it is not possible to know if the sharer server is temporarily unavailable or gone forever. Signed-off-by: Daniel Calviño Sánchez --- apps/files_sharing/lib/External/Manager.php | 32 +++++++++++++++++++-- apps/files_sharing/lib/External/Storage.php | 4 +-- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/apps/files_sharing/lib/External/Manager.php b/apps/files_sharing/lib/External/Manager.php index f18d8346dc4..77f5c6bd172 100644 --- a/apps/files_sharing/lib/External/Manager.php +++ b/apps/files_sharing/lib/External/Manager.php @@ -599,7 +599,18 @@ class Manager { return $result; } - public function removeShare($mountPoint): bool { + /** + * Remove a share based on its mountpoint. + * + * User shares are always fully removed; group shares by default are just + * marked again as pending for the current user, unless explicitly forced + * to be fully removed (parent group share and all its sub-shares). + * + * @param bool $force True to fully removed a group share, false to mark it + * as pending for the current user + * @return bool True if the share could be removed, false otherwise + */ + public function removeShare(string $mountPoint, bool $force = false): bool { try { $mountPointObj = $this->mountManager->find($mountPoint); } catch (NotFoundException $e) { @@ -617,7 +628,7 @@ class Manager { try { $getShare = $this->connection->prepare(' - SELECT `remote`, `share_token`, `remote_id`, `share_type`, `id` + SELECT `remote`, `share_token`, `remote_id`, `share_type`, `id`, `parent` FROM `*PREFIX*share_external` WHERE `mountpoint_hash` = ? AND `user` = ?'); $result = $getShare->execute([$hash, $this->uid]); @@ -638,7 +649,22 @@ class Manager { $deleteResult = $query->execute([(int)$share['id']]); $deleteResult->closeCursor(); } elseif ($share !== false && (int)$share['share_type'] === IShare::TYPE_GROUP) { - $this->updateAccepted((int)$share['id'], false); + if ($force) { + $qb = $this->connection->getQueryBuilder(); + // delete group share entry and matching sub-entries + $qb->delete('share_external') + ->where( + $qb->expr()->orX( + $qb->expr()->eq('id', $qb->createParameter('share_parent_id')), + $qb->expr()->eq('parent', $qb->createParameter('share_parent_id')) + ) + ); + + $qb->setParameter('share_parent_id', $share['parent']); + $qb->executeStatement(); + } else { + $this->updateAccepted((int)$share['id'], false); + } } $this->removeReShares($id); diff --git a/apps/files_sharing/lib/External/Storage.php b/apps/files_sharing/lib/External/Storage.php index 7b64690d53e..425cf178c9b 100644 --- a/apps/files_sharing/lib/External/Storage.php +++ b/apps/files_sharing/lib/External/Storage.php @@ -239,7 +239,7 @@ class Storage extends DAV implements ISharedStorage, IDisableEncryptionStorage, // valid Nextcloud instance means that the public share no longer exists // since this is permanent (re-sharing the file will create a new token) // we remove the invalid storage - $this->manager->removeShare($this->mountPoint); + $this->manager->removeShare($this->mountPoint, true); $this->manager->getMountManager()->removeMount($this->mountPoint); throw new StorageInvalidException("Remote share not found", 0, $e); } else { @@ -248,7 +248,7 @@ class Storage extends DAV implements ISharedStorage, IDisableEncryptionStorage, } } catch (ForbiddenException $e) { // auth error, remove share for now (provide a dialog in the future) - $this->manager->removeShare($this->mountPoint); + $this->manager->removeShare($this->mountPoint, true); $this->manager->getMountManager()->removeMount($this->mountPoint); throw new StorageInvalidException("Auth error when getting remote share"); } catch (\GuzzleHttp\Exception\ConnectException $e) {