Fix federated reshares

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();

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();

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();
 		}

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)))

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 [];
 	}
 

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);