diff --git a/apps/dav/appinfo/v1/publicwebdav.php b/apps/dav/appinfo/v1/publicwebdav.php index 2eaf05ca2d9b6..7b576496cc89f 100644 --- a/apps/dav/appinfo/v1/publicwebdav.php +++ b/apps/dav/appinfo/v1/publicwebdav.php @@ -92,7 +92,6 @@ function (\Sabre\DAV\Server $server) use ( } $share = $authBackend->getShare(); - $owner = $share->getShareOwner(); $isReadable = $share->getPermissions() & Constants::PERMISSION_READ; $fileId = $share->getNodeId(); @@ -107,7 +106,7 @@ function (\Sabre\DAV\Server $server) use ( 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 facc4041bccde..d1286a577c741 100644 --- a/apps/dav/appinfo/v2/publicremote.php +++ b/apps/dav/appinfo/v2/publicremote.php @@ -99,7 +99,6 @@ } $share = $authBackend->getShare(); - $owner = $share->getShareOwner(); $isReadable = $share->getPermissions() & Constants::PERMISSION_READ; $fileId = $share->getNodeId(); @@ -135,7 +134,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 c3b269fb9ed43..11b328d219492 100644 --- a/apps/dav/lib/Files/Sharing/PublicLinkCheckPlugin.php +++ b/apps/dav/lib/Files/Sharing/PublicLinkCheckPlugin.php @@ -43,7 +43,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 1d1c00772c5bf..842c7365f2367 100644 --- a/apps/federatedfilesharing/lib/FederatedShareProvider.php +++ b/apps/federatedfilesharing/lib/FederatedShareProvider.php @@ -128,30 +128,23 @@ public function create(IShare $share): IShare { $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); - } + $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->askOwnerToReShare($share->getShareOwner(), $share->getTarget(), $shareId, $shareWith, $permissions, $share->getNode()->getName(), $shareType); + // remote share was create successfully if we get a valid token as return + $send = is_string($token) && $token !== ''; + if ($send) { $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); } @@ -216,7 +209,7 @@ protected function createFederatedShare(IShare $share): string { } 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); @@ -225,15 +218,8 @@ protected function createFederatedShare(IShare $share): string { 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); + protected function askOwnerToReShare(string $owner, string $target, string $shareId, string $shareWith, int $permissions, string $name, int $shareType) { + $remoteShare = $this->getShareFromExternalShareTable($owner, $target); $token = $remoteShare['share_token']; $remoteId = $remoteShare['remote_id']; $remote = $remoteShare['remote']; @@ -244,9 +230,9 @@ protected function askOwnerToReShare($shareWith, IShare $share, $shareId) { $shareId, $remote, $shareWith, - $share->getPermissions(), - $share->getNode()->getName(), - $share->getShareType(), + $permissions, + $name, + $shareType, ); return [$token, $remoteId]; @@ -255,15 +241,14 @@ protected function askOwnerToReShare($shareWith, IShare $share, $shareId) { /** * 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->fetchAllAssociative(); $qResult->closeCursor(); @@ -453,7 +438,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()); } /** @@ -483,19 +468,10 @@ 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. */ - private function removeShareFromTableById(string $shareId): void { + 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 a5ae8c87abd25..3ae6366fabe63 100644 --- a/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php +++ b/apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php @@ -382,7 +382,7 @@ protected function shareDeclined(string $id, array $notification): array { * @throws ShareNotFound */ protected function executeDeclineShare(IShare $share): void { - $this->federatedShareProvider->removeShareFromTable($share); + $this->federatedShareProvider->removeShareFromTable($share->getId()); $user = $this->getCorrectUser($share); @@ -420,7 +420,7 @@ private function undoReshare(string $id, array $notification): array { $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 0852a4b548904..0d5c9be18ffdd 100644 --- a/apps/federatedfilesharing/tests/FederatedShareProviderTest.php +++ b/apps/federatedfilesharing/tests/FederatedShareProviderTest.php @@ -133,7 +133,8 @@ public function testCreate(?\DateTime $expirationDate, ?string $expectedDataDate ->setPermissions(19) ->setShareType(IShare::TYPE_REMOTE) ->setExpirationDate($expirationDate) - ->setNode($node); + ->setNode($node) + ->setTarget(''); $this->tokenHandler->method('generateToken')->willReturn('token'); @@ -215,7 +216,8 @@ public function testCreateCouldNotFindServer(): void { ->setShareOwner('shareOwner') ->setPermissions(19) ->setShareType(IShare::TYPE_REMOTE) - ->setNode($node); + ->setNode($node) + ->setTarget(''); $this->tokenHandler->method('generateToken')->willReturn('token'); @@ -268,7 +270,8 @@ public function testCreateException(): void { ->setShareOwner('shareOwner') ->setPermissions(19) ->setShareType(IShare::TYPE_REMOTE) - ->setNode($node); + ->setNode($node) + ->setTarget(''); $this->tokenHandler->method('generateToken')->willReturn('token'); @@ -359,7 +362,8 @@ public function testCreateAlreadyShared(): void { ->setShareOwner('shareOwner') ->setPermissions(19) ->setShareType(IShare::TYPE_REMOTE) - ->setNode($node); + ->setNode($node) + ->setTarget(''); $this->tokenHandler->method('generateToken')->willReturn('token'); @@ -431,7 +435,8 @@ public function testUpdate(string $owner, string $sharedBy, ?\DateTime $expirati ->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') @@ -508,7 +513,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(); @@ -517,7 +523,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); @@ -552,7 +559,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(); @@ -565,7 +573,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); @@ -599,7 +608,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(); @@ -608,7 +618,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); @@ -649,7 +660,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(); @@ -658,7 +670,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); @@ -839,7 +852,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(); @@ -848,7 +862,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); @@ -899,7 +914,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(); @@ -908,7 +924,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);