From 9dc25bc44f494913be164c4782fd56991dccb189 Mon Sep 17 00:00:00 2001 From: provokateurin Date: Tue, 3 Mar 2026 15:25:03 +0100 Subject: [PATCH 1/2] fix(federatedfilesharing): Do not set the share id for an existing share Signed-off-by: provokateurin --- .../lib/FederatedShareProvider.php | 63 ++++++++----------- .../lib/OCM/CloudFederationProviderFiles.php | 6 +- .../tests/FederatedShareProviderTest.php | 51 ++++++++++----- 3 files changed, 64 insertions(+), 56 deletions(-) diff --git a/apps/federatedfilesharing/lib/FederatedShareProvider.php b/apps/federatedfilesharing/lib/FederatedShareProvider.php index 29983c7981255..3a6b3e717f027 100644 --- a/apps/federatedfilesharing/lib/FederatedShareProvider.php +++ b/apps/federatedfilesharing/lib/FederatedShareProvider.php @@ -130,30 +130,30 @@ public function create(IShare $share) { $share->setSharedWith($cloudId->getId()); try { - $remoteShare = $this->getShareFromExternalShareTable($share); + $remoteShare = $this->getShareFromExternalShareTable($share->getShareOwner(), $share->getTarget()); } catch (ShareNotFound $e) { $remoteShare = null; } if ($remoteShare) { - try { - $ownerCloudId = $this->cloudIdManager->getCloudId($remoteShare['owner'], $remoteShare['remote']); - $shareId = $this->addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $ownerCloudId->getId(), $permissions, 'tmp_token_' . time(), $shareType, $expirationDate); - $share->setId($shareId); - [$token, $remoteId] = $this->askOwnerToReShare($shareWith, $share, $shareId); - // remote share was create successfully if we get a valid token as return - $send = is_string($token) && $token !== ''; - } catch (\Exception $e) { - // fall back to old re-share behavior if the remote server - // doesn't support flat re-shares (was introduced with Nextcloud 9.1) - $this->removeShareFromTable($share); - $shareId = $this->createFederatedShare($share); - } - if ($send) { + $ownerCloudId = $this->cloudIdManager->getCloudId($remoteShare['owner'], $remoteShare['remote']); + $shareId = $this->addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $ownerCloudId->getId(), $permissions, 'tmp_token_' . time(), $shareType, $expirationDate); + [$token, $remoteId] = $this->notifications->requestReShare( + $remoteShare['share_token'], + $remoteShare['remote_id'], + $shareId, + $remoteShare['remote'], + $shareWith, + $permissions, + $share->getNode()->getName(), + $shareType, + ); + // remote share was create successfully if we get a valid token as return + if (is_string($token) && $token !== '') { $this->updateSuccessfulReshare($shareId, $token); $this->storeRemoteId($shareId, $remoteId); } else { - $this->removeShareFromTable($share); + $this->removeShareFromTable($shareId); $message_t = $this->l->t('File is already shared with %s', [$shareWith]); throw new \Exception($message_t); } @@ -220,7 +220,7 @@ protected function createFederatedShare(IShare $share) { } if ($failure) { - $this->removeShareFromTableById($shareId); + $this->removeShareFromTable($shareId); $message_t = $this->l->t('Sharing %1$s failed, could not find %2$s, maybe the server is currently unreachable or uses a self-signed certificate.', [$share->getNode()->getName(), $share->getSharedWith()]); throw new \Exception($message_t); @@ -230,6 +230,7 @@ protected function createFederatedShare(IShare $share) { } /** +<<<<<<< HEAD * @param string $shareWith * @param IShare $share * @param string $shareId internal share Id @@ -256,18 +257,19 @@ protected function askOwnerToReShare($shareWith, IShare $share, $shareId) { } /** +======= +>>>>>>> 045ad43237c (fix(federatedfilesharing): Do not set the share id for an existing share) * get federated share from the share_external table but exclude mounted link shares * - * @param IShare $share * @return array * @throws ShareNotFound */ - protected function getShareFromExternalShareTable(IShare $share) { + protected function getShareFromExternalShareTable(string $owner, string $target) { $query = $this->dbConnection->getQueryBuilder(); $query->select('*')->from($this->externalShareTable) - ->where($query->expr()->eq('user', $query->createNamedParameter($share->getShareOwner()))) - ->andWhere($query->expr()->eq('mountpoint', $query->createNamedParameter($share->getTarget()))); - $qResult = $query->execute(); + ->where($query->expr()->eq('user', $query->createNamedParameter($owner))) + ->andWhere($query->expr()->eq('mountpoint', $query->createNamedParameter($target))); + $qResult = $query->executeQuery(); $result = $qResult->fetchAll(); $qResult->closeCursor(); @@ -475,7 +477,7 @@ public function delete(IShare $share) { // only remove the share when all messages are send to not lose information // about the share to early - $this->removeShareFromTable($share); + $this->removeShareFromTable($share->getId()); } /** @@ -506,20 +508,9 @@ protected function revokeShare($share, $isOwner) { } /** - * remove share from table - * - * @param IShare $share - */ - public function removeShareFromTable(IShare $share) { - $this->removeShareFromTableById($share->getId()); - } - - /** - * remove share from table - * - * @param string $shareId + * Remove share from table. */ - private function removeShareFromTableById($shareId) { + public function removeShareFromTable(string $shareId): void { $qb = $this->dbConnection->getQueryBuilder(); $qb->delete('share') ->where($qb->expr()->eq('id', $qb->createNamedParameter($shareId))) diff --git a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php index 091f2b201c1c8..b4475ab4bc976 100644 --- a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php +++ b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php @@ -382,8 +382,8 @@ protected function shareDeclined($id, array $notification) { * @param IShare $share * @throws ShareNotFound */ - protected function executeDeclineShare(IShare $share) { - $this->federatedShareProvider->removeShareFromTable($share); + protected function executeDeclineShare(IShare $share): void { + $this->federatedShareProvider->removeShareFromTable($share->getId()); try { $fileId = (int) $share->getNode()->getId(); @@ -420,7 +420,7 @@ private function undoReshare($id, array $notification) { $share = $this->federatedShareProvider->getShareById($id); $this->verifyShare($share, $token); - $this->federatedShareProvider->removeShareFromTable($share); + $this->federatedShareProvider->removeShareFromTable($share->getId()); return []; } diff --git a/apps/federatedfilesharing/tests/FederatedShareProviderTest.php b/apps/federatedfilesharing/tests/FederatedShareProviderTest.php index 72a7b2adeb872..3f4d24a9ce146 100644 --- a/apps/federatedfilesharing/tests/FederatedShareProviderTest.php +++ b/apps/federatedfilesharing/tests/FederatedShareProviderTest.php @@ -153,7 +153,8 @@ public function testCreate($expirationDate, $expectedDataDate) { ->setPermissions(19) ->setShareType(IShare::TYPE_REMOTE) ->setExpirationDate($expirationDate) - ->setNode($node); + ->setNode($node) + ->setTarget(''); $this->tokenHandler->method('generateToken')->willReturn('token'); @@ -234,7 +235,8 @@ public function testCreateCouldNotFindServer() { ->setShareOwner('shareOwner') ->setPermissions(19) ->setShareType(IShare::TYPE_REMOTE) - ->setNode($node); + ->setNode($node) + ->setTarget(''); $this->tokenHandler->method('generateToken')->willReturn('token'); @@ -295,7 +297,8 @@ public function testCreateException() { ->setShareOwner('shareOwner') ->setPermissions(19) ->setShareType(IShare::TYPE_REMOTE) - ->setNode($node); + ->setNode($node) + ->setTarget(''); $this->tokenHandler->method('generateToken')->willReturn('token'); @@ -403,7 +406,8 @@ public function testCreateAlreadyShared() { ->setShareOwner('shareOwner') ->setPermissions(19) ->setShareType(IShare::TYPE_REMOTE) - ->setNode($node); + ->setNode($node) + ->setTarget(''); $this->tokenHandler->method('generateToken')->willReturn('token'); @@ -475,7 +479,8 @@ public function testUpdate($owner, $sharedBy, $expirationDate) { ->setPermissions(19) ->setShareType(IShare::TYPE_REMOTE) ->setExpirationDate(new \DateTime('2019-02-01T01:02:03')) - ->setNode($node); + ->setNode($node) + ->setTarget(''); $this->tokenHandler->method('generateToken')->willReturn('token'); $this->addressHandler->expects($this->any())->method('generateRemoteURL') @@ -552,7 +557,8 @@ public function testGetSharedBy() { ->setShareOwner('shareOwner') ->setPermissions(19) ->setShareType(IShare::TYPE_REMOTE) - ->setNode($node); + ->setNode($node) + ->setTarget(''); $this->provider->create($share); $share2 = $this->shareManager->newShare(); @@ -561,7 +567,8 @@ public function testGetSharedBy() { ->setShareOwner('shareOwner') ->setPermissions(19) ->setShareType(IShare::TYPE_REMOTE) - ->setNode($node); + ->setNode($node) + ->setTarget(''); $this->provider->create($share2); $shares = $this->provider->getSharesBy('sharedBy', IShare::TYPE_REMOTE, null, false, -1, 0); @@ -596,7 +603,8 @@ public function testGetSharedByWithNode() { ->setShareOwner('shareOwner') ->setPermissions(19) ->setShareType(IShare::TYPE_REMOTE) - ->setNode($node); + ->setNode($node) + ->setTarget(''); $this->provider->create($share); $node2 = $this->getMockBuilder(File::class)->getMock(); @@ -609,7 +617,8 @@ public function testGetSharedByWithNode() { ->setShareOwner('shareOwner') ->setPermissions(19) ->setShareType(IShare::TYPE_REMOTE) - ->setNode($node2); + ->setNode($node2) + ->setTarget(''); $this->provider->create($share2); $shares = $this->provider->getSharesBy('sharedBy', IShare::TYPE_REMOTE, $node2, false, -1, 0); @@ -643,7 +652,8 @@ public function testGetSharedByWithReshares() { ->setShareOwner('shareOwner') ->setPermissions(19) ->setShareType(IShare::TYPE_REMOTE) - ->setNode($node); + ->setNode($node) + ->setTarget(''); $this->provider->create($share); $share2 = $this->shareManager->newShare(); @@ -652,7 +662,8 @@ public function testGetSharedByWithReshares() { ->setShareOwner('shareOwner') ->setPermissions(19) ->setShareType(IShare::TYPE_REMOTE) - ->setNode($node); + ->setNode($node) + ->setTarget(''); $this->provider->create($share2); $shares = $this->provider->getSharesBy('shareOwner', IShare::TYPE_REMOTE, null, true, -1, 0); @@ -693,7 +704,8 @@ public function testGetSharedByWithLimit() { ->setShareOwner('shareOwner') ->setPermissions(19) ->setShareType(IShare::TYPE_REMOTE) - ->setNode($node); + ->setNode($node) + ->setTarget(''); $this->provider->create($share); $share2 = $this->shareManager->newShare(); @@ -702,7 +714,8 @@ public function testGetSharedByWithLimit() { ->setShareOwner('shareOwner') ->setPermissions(19) ->setShareType(IShare::TYPE_REMOTE) - ->setNode($node); + ->setNode($node) + ->setTarget(''); $this->provider->create($share2); $shares = $this->provider->getSharesBy('shareOwner', IShare::TYPE_REMOTE, null, true, 1, 1); @@ -903,7 +916,8 @@ public function testGetSharesInFolder() { ->setShareOwner($u1->getUID()) ->setPermissions(\OCP\Constants::PERMISSION_READ) ->setShareType(IShare::TYPE_REMOTE) - ->setNode($file1); + ->setNode($file1) + ->setTarget(''); $this->provider->create($share1); $share2 = $this->shareManager->newShare(); @@ -912,7 +926,8 @@ public function testGetSharesInFolder() { ->setShareOwner($u1->getUID()) ->setPermissions(\OCP\Constants::PERMISSION_READ) ->setShareType(IShare::TYPE_REMOTE) - ->setNode($file2); + ->setNode($file2) + ->setTarget(''); $this->provider->create($share2); $result = $this->provider->getSharesInFolder($u1->getUID(), $folder1, false); @@ -963,7 +978,8 @@ public function testGetAccessList() { ->setShareOwner($u1->getUID()) ->setPermissions(\OCP\Constants::PERMISSION_READ) ->setShareType(IShare::TYPE_REMOTE) - ->setNode($file1); + ->setNode($file1) + ->setTarget(''); $this->provider->create($share1); $share2 = $this->shareManager->newShare(); @@ -972,7 +988,8 @@ public function testGetAccessList() { ->setShareOwner($u1->getUID()) ->setPermissions(\OCP\Constants::PERMISSION_READ) ->setShareType(IShare::TYPE_REMOTE) - ->setNode($file1); + ->setNode($file1) + ->setTarget(''); $this->provider->create($share2); $result = $this->provider->getAccessList([$file1], true); From 3ed0dcca5d86b30122596fc087be5ad6ee90bd52 Mon Sep 17 00:00:00 2001 From: provokateurin Date: Tue, 3 Mar 2026 15:25:54 +0100 Subject: [PATCH 2/2] fix(dav): Use share initiator to get the share node, because the owner might on another server Signed-off-by: provokateurin --- apps/dav/appinfo/v1/publicwebdav.php | 3 +-- apps/dav/appinfo/v2/publicremote.php | 3 +-- apps/dav/lib/Files/Sharing/PublicLinkCheckPlugin.php | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/apps/dav/appinfo/v1/publicwebdav.php b/apps/dav/appinfo/v1/publicwebdav.php index 3875337415004..c975d78b46f56 100644 --- a/apps/dav/appinfo/v1/publicwebdav.php +++ b/apps/dav/appinfo/v1/publicwebdav.php @@ -58,7 +58,6 @@ } $share = $authBackend->getShare(); - $owner = $share->getShareOwner(); $isReadable = $share->getPermissions() & \OCP\Constants::PERMISSION_READ; $fileId = $share->getNodeId(); @@ -73,7 +72,7 @@ \OC\Files\Filesystem::logWarningWhenAddingStorageWrapper($previousLog); $rootFolder = \OCP\Server::get(\OCP\Files\IRootFolder::class); - $userFolder = $rootFolder->getUserFolder($owner); + $userFolder = $rootFolder->getUserFolder($share->getSharedBy()); $node = $userFolder->getFirstNodeById($fileId); if (!$node) { throw new \Sabre\DAV\Exception\NotFound(); diff --git a/apps/dav/appinfo/v2/publicremote.php b/apps/dav/appinfo/v2/publicremote.php index 0b7480872cb6a..c59196f728d80 100644 --- a/apps/dav/appinfo/v2/publicremote.php +++ b/apps/dav/appinfo/v2/publicremote.php @@ -85,7 +85,6 @@ } $share = $authBackend->getShare(); - $owner = $share->getShareOwner(); $isReadable = $share->getPermissions() & \OCP\Constants::PERMISSION_READ; $fileId = $share->getNodeId(); @@ -113,7 +112,7 @@ Filesystem::logWarningWhenAddingStorageWrapper($previousLog); $rootFolder = \OCP\Server::get(\OCP\Files\IRootFolder::class); - $userFolder = $rootFolder->getUserFolder($owner); + $userFolder = $rootFolder->getUserFolder($share->getSharedBy()); $node = $userFolder->getFirstNodeById($fileId); if (!$node) { throw new NotFound(); diff --git a/apps/dav/lib/Files/Sharing/PublicLinkCheckPlugin.php b/apps/dav/lib/Files/Sharing/PublicLinkCheckPlugin.php index 5f1e29e0286d6..daabad80e2173 100644 --- a/apps/dav/lib/Files/Sharing/PublicLinkCheckPlugin.php +++ b/apps/dav/lib/Files/Sharing/PublicLinkCheckPlugin.php @@ -41,7 +41,7 @@ public function initialize(\Sabre\DAV\Server $server) { } public function beforeMethod(RequestInterface $request, ResponseInterface $response) { - // verify that the owner didn't have his share permissions revoked + // verify that the initiator didn't have their share permissions revoked if ($this->fileInfo && !$this->fileInfo->isShareable()) { throw new NotFound(); }