diff --git a/apps/dav/appinfo/v1/publicwebdav.php b/apps/dav/appinfo/v1/publicwebdav.php index dc74fe214afbb..94e3ccc80017e 100644 --- a/apps/dav/appinfo/v1/publicwebdav.php +++ b/apps/dav/appinfo/v1/publicwebdav.php @@ -69,7 +69,6 @@ } $share = $authBackend->getShare(); - $owner = $share->getShareOwner(); $isReadable = $share->getPermissions() & Constants::PERMISSION_READ; $fileId = $share->getNodeId(); @@ -84,7 +83,7 @@ Filesystem::logWarningWhenAddingStorageWrapper($previousLog); $rootFolder = Server::get(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 e32f7efb37445..31b7c80d31ac3 100644 --- a/apps/dav/appinfo/v2/publicremote.php +++ b/apps/dav/appinfo/v2/publicremote.php @@ -95,7 +95,6 @@ } $share = $authBackend->getShare(); - $owner = $share->getShareOwner(); $isReadable = $share->getPermissions() & Constants::PERMISSION_READ; $fileId = $share->getNodeId(); @@ -123,7 +122,7 @@ Filesystem::logWarningWhenAddingStorageWrapper($previousLog); $rootFolder = Server::get(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 38a45b3fc37e4..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 their share permissions revoked + // verify that the initiator didn't have their share permissions revoked if ($this->fileInfo && !$this->fileInfo->isShareable()) { throw new NotFound(); } diff --git a/apps/federatedfilesharing/lib/FederatedShareProvider.php b/apps/federatedfilesharing/lib/FederatedShareProvider.php index 5292291a0cc8b..795dfa6d75b45 100644 --- a/apps/federatedfilesharing/lib/FederatedShareProvider.php +++ b/apps/federatedfilesharing/lib/FederatedShareProvider.php @@ -132,30 +132,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); } @@ -222,7 +222,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); @@ -231,45 +231,17 @@ protected function createFederatedShare(IShare $share) { return $shareId; } - /** - * @param string $shareWith - * @param IShare $share - * @param string $shareId internal share Id - * @return array - * @throws \Exception - */ - protected function askOwnerToReShare($shareWith, IShare $share, $shareId) { - $remoteShare = $this->getShareFromExternalShareTable($share); - $token = $remoteShare['share_token']; - $remoteId = $remoteShare['remote_id']; - $remote = $remoteShare['remote']; - - [$token, $remoteId] = $this->notifications->requestReShare( - $token, - $remoteId, - $shareId, - $remote, - $shareWith, - $share->getPermissions(), - $share->getNode()->getName(), - $share->getShareType(), - ); - - return [$token, $remoteId]; - } - /** * 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()))); + ->where($query->expr()->eq('user', $query->createNamedParameter($owner))) + ->andWhere($query->expr()->eq('mountpoint', $query->createNamedParameter($target))); $qResult = $query->executeQuery(); $result = $qResult->fetchAll(); $qResult->closeCursor(); @@ -478,7 +450,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()); } /** @@ -509,20 +481,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 a240d488305f4..f39c5eb209df3 100644 --- a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php +++ b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php @@ -409,8 +409,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(); @@ -447,7 +447,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 b9bc6309148e4..912fb1bf463e3 100644 --- a/apps/federatedfilesharing/tests/FederatedShareProviderTest.php +++ b/apps/federatedfilesharing/tests/FederatedShareProviderTest.php @@ -154,7 +154,8 @@ public function testCreate($expirationDate, $expectedDataDate): void { ->setPermissions(19) ->setShareType(IShare::TYPE_REMOTE) ->setExpirationDate($expirationDate) - ->setNode($node); + ->setNode($node) + ->setTarget(''); $this->tokenHandler->method('generateToken')->willReturn('token'); @@ -235,7 +236,8 @@ public function testCreateCouldNotFindServer(): void { ->setShareOwner('shareOwner') ->setPermissions(19) ->setShareType(IShare::TYPE_REMOTE) - ->setNode($node); + ->setNode($node) + ->setTarget(''); $this->tokenHandler->method('generateToken')->willReturn('token'); @@ -296,7 +298,8 @@ public function testCreateException(): void { ->setShareOwner('shareOwner') ->setPermissions(19) ->setShareType(IShare::TYPE_REMOTE) - ->setNode($node); + ->setNode($node) + ->setTarget(''); $this->tokenHandler->method('generateToken')->willReturn('token'); @@ -404,7 +407,8 @@ public function testCreateAlreadyShared(): void { ->setShareOwner('shareOwner') ->setPermissions(19) ->setShareType(IShare::TYPE_REMOTE) - ->setNode($node); + ->setNode($node) + ->setTarget(''); $this->tokenHandler->method('generateToken')->willReturn('token'); @@ -476,7 +480,8 @@ public function testUpdate($owner, $sharedBy, $expirationDate): void { ->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') @@ -553,7 +558,8 @@ public function testGetSharedBy(): void { ->setShareOwner('shareOwner') ->setPermissions(19) ->setShareType(IShare::TYPE_REMOTE) - ->setNode($node); + ->setNode($node) + ->setTarget(''); $this->provider->create($share); $share2 = $this->shareManager->newShare(); @@ -562,7 +568,8 @@ public function testGetSharedBy(): void { ->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); @@ -597,7 +604,8 @@ public function testGetSharedByWithNode(): void { ->setShareOwner('shareOwner') ->setPermissions(19) ->setShareType(IShare::TYPE_REMOTE) - ->setNode($node); + ->setNode($node) + ->setTarget(''); $this->provider->create($share); $node2 = $this->getMockBuilder(File::class)->getMock(); @@ -610,7 +618,8 @@ public function testGetSharedByWithNode(): void { ->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); @@ -644,7 +653,8 @@ public function testGetSharedByWithReshares(): void { ->setShareOwner('shareOwner') ->setPermissions(19) ->setShareType(IShare::TYPE_REMOTE) - ->setNode($node); + ->setNode($node) + ->setTarget(''); $this->provider->create($share); $share2 = $this->shareManager->newShare(); @@ -653,7 +663,8 @@ public function testGetSharedByWithReshares(): void { ->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); @@ -694,7 +705,8 @@ public function testGetSharedByWithLimit(): void { ->setShareOwner('shareOwner') ->setPermissions(19) ->setShareType(IShare::TYPE_REMOTE) - ->setNode($node); + ->setNode($node) + ->setTarget(''); $this->provider->create($share); $share2 = $this->shareManager->newShare(); @@ -703,7 +715,8 @@ public function testGetSharedByWithLimit(): void { ->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); @@ -904,7 +917,8 @@ public function testGetSharesInFolder(): void { ->setShareOwner($u1->getUID()) ->setPermissions(Constants::PERMISSION_READ) ->setShareType(IShare::TYPE_REMOTE) - ->setNode($file1); + ->setNode($file1) + ->setTarget(''); $this->provider->create($share1); $share2 = $this->shareManager->newShare(); @@ -913,7 +927,8 @@ public function testGetSharesInFolder(): void { ->setShareOwner($u1->getUID()) ->setPermissions(Constants::PERMISSION_READ) ->setShareType(IShare::TYPE_REMOTE) - ->setNode($file2); + ->setNode($file2) + ->setTarget(''); $this->provider->create($share2); $result = $this->provider->getSharesInFolder($u1->getUID(), $folder1, false); @@ -964,7 +979,8 @@ public function testGetAccessList(): void { ->setShareOwner($u1->getUID()) ->setPermissions(Constants::PERMISSION_READ) ->setShareType(IShare::TYPE_REMOTE) - ->setNode($file1); + ->setNode($file1) + ->setTarget(''); $this->provider->create($share1); $share2 = $this->shareManager->newShare(); @@ -973,7 +989,8 @@ public function testGetAccessList(): void { ->setShareOwner($u1->getUID()) ->setPermissions(Constants::PERMISSION_READ) ->setShareType(IShare::TYPE_REMOTE) - ->setNode($file1); + ->setNode($file1) + ->setTarget(''); $this->provider->create($share2); $result = $this->provider->getAccessList([$file1], true);