fix(federatedfilesharing): Do not set the share id for an existing share

Signed-off-by: provokateurin <kate@provokateurin.de>
This commit is contained in:
provokateurin 2026-03-03 15:25:03 +01:00
parent 6a74a829d0
commit 5e6f099624
No known key found for this signature in database
3 changed files with 61 additions and 83 deletions

View file

@ -130,30 +130,29 @@ class FederatedShareProvider implements IShareProvider {
$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(),
);
// 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 +219,7 @@ class FederatedShareProvider implements IShareProvider {
}
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);
@ -229,45 +228,18 @@ class FederatedShareProvider implements IShareProvider {
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()
);
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())));
$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();
@ -289,7 +261,7 @@ class FederatedShareProvider implements IShareProvider {
* @param int $permissions
* @param string $token
* @param int $shareType
* @param \DateTime $expirationDate
* @param ?\DateTime $expirationDate
* @return int
*/
private function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $uidOwner, $permissions, $token, $shareType, $expirationDate) {
@ -475,7 +447,7 @@ class FederatedShareProvider implements IShareProvider {
// only remove the share when all messages are send to not lose information
// about the share to early
$this->removeShareFromTable($share);
$this->removeShareFromTable((int) $share->getId());
}
/**
@ -506,20 +478,9 @@ class FederatedShareProvider implements IShareProvider {
}
/**
* remove share from table
*
* @param IShare $share
* Remove share from table.
*/
public function removeShareFromTable(IShare $share) {
$this->removeShareFromTableById($share->getId());
}
/**
* remove share from table
*
* @param string $shareId
*/
private function removeShareFromTableById($shareId) {
public function removeShareFromTable(int $shareId): void {
$qb = $this->dbConnection->getQueryBuilder();
$qb->delete('share')
->where($qb->expr()->eq('id', $qb->createNamedParameter($shareId)))

View file

@ -382,8 +382,8 @@ class CloudFederationProviderFiles implements ICloudFederationProvider {
* @param IShare $share
* @throws ShareNotFound
*/
protected function executeDeclineShare(IShare $share) {
$this->federatedShareProvider->removeShareFromTable($share);
protected function executeDeclineShare(IShare $share): void {
$this->federatedShareProvider->removeShareFromTable((int) $share->getId());
try {
$fileId = (int) $share->getNode()->getId();
@ -420,7 +420,7 @@ class CloudFederationProviderFiles implements ICloudFederationProvider {
$share = $this->federatedShareProvider->getShareById($id);
$this->verifyShare($share, $token);
$this->federatedShareProvider->removeShareFromTable($share);
$this->federatedShareProvider->removeShareFromTable((int) $share->getId());
return [];
}

View file

@ -153,7 +153,8 @@ class FederatedShareProviderTest extends \Test\TestCase {
->setPermissions(19)
->setShareType(IShare::TYPE_REMOTE)
->setExpirationDate($expirationDate)
->setNode($node);
->setNode($node)
->setTarget('');
$this->tokenHandler->method('generateToken')->willReturn('token');
@ -234,7 +235,8 @@ class FederatedShareProviderTest extends \Test\TestCase {
->setShareOwner('shareOwner')
->setPermissions(19)
->setShareType(IShare::TYPE_REMOTE)
->setNode($node);
->setNode($node)
->setTarget('');
$this->tokenHandler->method('generateToken')->willReturn('token');
@ -295,7 +297,8 @@ class FederatedShareProviderTest extends \Test\TestCase {
->setShareOwner('shareOwner')
->setPermissions(19)
->setShareType(IShare::TYPE_REMOTE)
->setNode($node);
->setNode($node)
->setTarget('');
$this->tokenHandler->method('generateToken')->willReturn('token');
@ -403,7 +406,8 @@ class FederatedShareProviderTest extends \Test\TestCase {
->setShareOwner('shareOwner')
->setPermissions(19)
->setShareType(IShare::TYPE_REMOTE)
->setNode($node);
->setNode($node)
->setTarget('');
$this->tokenHandler->method('generateToken')->willReturn('token');
@ -475,7 +479,8 @@ class FederatedShareProviderTest extends \Test\TestCase {
->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 @@ class FederatedShareProviderTest extends \Test\TestCase {
->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 @@ class FederatedShareProviderTest extends \Test\TestCase {
->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 @@ class FederatedShareProviderTest extends \Test\TestCase {
->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 @@ class FederatedShareProviderTest extends \Test\TestCase {
->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 @@ class FederatedShareProviderTest extends \Test\TestCase {
->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 @@ class FederatedShareProviderTest extends \Test\TestCase {
->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 @@ class FederatedShareProviderTest extends \Test\TestCase {
->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 @@ class FederatedShareProviderTest extends \Test\TestCase {
->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 @@ class FederatedShareProviderTest extends \Test\TestCase {
->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 @@ class FederatedShareProviderTest extends \Test\TestCase {
->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 @@ class FederatedShareProviderTest extends \Test\TestCase {
->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 @@ class FederatedShareProviderTest extends \Test\TestCase {
->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);