Skip to content

Commit ca05a9a

Browse files
sorbaughsorbaugh
authored
Merge pull request #58901 from nextcloud/backport/58689/stable31
[stable31] Fix federated reshares
2 parents 7061625 + 824eb71 commit ca05a9a

6 files changed

Lines changed: 63 additions & 87 deletions

File tree

‎apps/dav/appinfo/v1/publicwebdav.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@
6969
}
7070

7171
$share = $authBackend->getShare();
72-
$owner = $share->getShareOwner();
7372
$isReadable = $share->getPermissions() & Constants::PERMISSION_READ;
7473
$fileId = $share->getNodeId();
7574

@@ -84,7 +83,7 @@
8483
Filesystem::logWarningWhenAddingStorageWrapper($previousLog);
8584

8685
$rootFolder = Server::get(IRootFolder::class);
87-
$userFolder = $rootFolder->getUserFolder($owner);
86+
$userFolder = $rootFolder->getUserFolder($share->getSharedBy());
8887
$node = $userFolder->getFirstNodeById($fileId);
8988
if (!$node) {
9089
throw new \Sabre\DAV\Exception\NotFound();

‎apps/dav/appinfo/v2/publicremote.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,6 @@
9595
}
9696

9797
$share = $authBackend->getShare();
98-
$owner = $share->getShareOwner();
9998
$isReadable = $share->getPermissions() & Constants::PERMISSION_READ;
10099
$fileId = $share->getNodeId();
101100

@@ -123,7 +122,7 @@
123122
Filesystem::logWarningWhenAddingStorageWrapper($previousLog);
124123

125124
$rootFolder = Server::get(IRootFolder::class);
126-
$userFolder = $rootFolder->getUserFolder($owner);
125+
$userFolder = $rootFolder->getUserFolder($share->getSharedBy());
127126
$node = $userFolder->getFirstNodeById($fileId);
128127
if (!$node) {
129128
throw new NotFound();

‎apps/dav/lib/Files/Sharing/PublicLinkCheckPlugin.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public function initialize(\Sabre\DAV\Server $server) {
4141
}
4242

4343
public function beforeMethod(RequestInterface $request, ResponseInterface $response) {
44-
// verify that the owner didn't have their share permissions revoked
44+
// verify that the initiator didn't have their share permissions revoked
4545
if ($this->fileInfo && !$this->fileInfo->isShareable()) {
4646
throw new NotFound();
4747
}

‎apps/federatedfilesharing/lib/FederatedShareProvider.php‎

Lines changed: 23 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -132,30 +132,30 @@ public function create(IShare $share) {
132132
$share->setSharedWith($cloudId->getId());
133133

134134
try {
135-
$remoteShare = $this->getShareFromExternalShareTable($share);
135+
$remoteShare = $this->getShareFromExternalShareTable($share->getShareOwner(), $share->getTarget());
136136
} catch (ShareNotFound $e) {
137137
$remoteShare = null;
138138
}
139139

140140
if ($remoteShare) {
141-
try {
142-
$ownerCloudId = $this->cloudIdManager->getCloudId($remoteShare['owner'], $remoteShare['remote']);
143-
$shareId = $this->addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $ownerCloudId->getId(), $permissions, 'tmp_token_' . time(), $shareType, $expirationDate);
144-
$share->setId($shareId);
145-
[$token, $remoteId] = $this->askOwnerToReShare($shareWith, $share, $shareId);
146-
// remote share was create successfully if we get a valid token as return
147-
$send = is_string($token) && $token !== '';
148-
} catch (\Exception $e) {
149-
// fall back to old re-share behavior if the remote server
150-
// doesn't support flat re-shares (was introduced with Nextcloud 9.1)
151-
$this->removeShareFromTable($share);
152-
$shareId = $this->createFederatedShare($share);
153-
}
154-
if ($send) {
141+
$ownerCloudId = $this->cloudIdManager->getCloudId($remoteShare['owner'], $remoteShare['remote']);
142+
$shareId = $this->addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $ownerCloudId->getId(), $permissions, 'tmp_token_' . time(), $shareType, $expirationDate);
143+
[$token, $remoteId] = $this->notifications->requestReShare(
144+
$remoteShare['share_token'],
145+
$remoteShare['remote_id'],
146+
$shareId,
147+
$remoteShare['remote'],
148+
$shareWith,
149+
$permissions,
150+
$share->getNode()->getName(),
151+
$shareType,
152+
);
153+
// remote share was create successfully if we get a valid token as return
154+
if (is_string($token) && $token !== '') {
155155
$this->updateSuccessfulReshare($shareId, $token);
156156
$this->storeRemoteId($shareId, $remoteId);
157157
} else {
158-
$this->removeShareFromTable($share);
158+
$this->removeShareFromTable($shareId);
159159
$message_t = $this->l->t('File is already shared with %s', [$shareWith]);
160160
throw new \Exception($message_t);
161161
}
@@ -222,7 +222,7 @@ protected function createFederatedShare(IShare $share) {
222222
}
223223

224224
if ($failure) {
225-
$this->removeShareFromTableById($shareId);
225+
$this->removeShareFromTable($shareId);
226226
$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.',
227227
[$share->getNode()->getName(), $share->getSharedWith()]);
228228
throw new \Exception($message_t);
@@ -231,45 +231,17 @@ protected function createFederatedShare(IShare $share) {
231231
return $shareId;
232232
}
233233

234-
/**
235-
* @param string $shareWith
236-
* @param IShare $share
237-
* @param string $shareId internal share Id
238-
* @return array
239-
* @throws \Exception
240-
*/
241-
protected function askOwnerToReShare($shareWith, IShare $share, $shareId) {
242-
$remoteShare = $this->getShareFromExternalShareTable($share);
243-
$token = $remoteShare['share_token'];
244-
$remoteId = $remoteShare['remote_id'];
245-
$remote = $remoteShare['remote'];
246-
247-
[$token, $remoteId] = $this->notifications->requestReShare(
248-
$token,
249-
$remoteId,
250-
$shareId,
251-
$remote,
252-
$shareWith,
253-
$share->getPermissions(),
254-
$share->getNode()->getName(),
255-
$share->getShareType(),
256-
);
257-
258-
return [$token, $remoteId];
259-
}
260-
261234
/**
262235
* get federated share from the share_external table but exclude mounted link shares
263236
*
264-
* @param IShare $share
265237
* @return array
266238
* @throws ShareNotFound
267239
*/
268-
protected function getShareFromExternalShareTable(IShare $share) {
240+
protected function getShareFromExternalShareTable(string $owner, string $target) {
269241
$query = $this->dbConnection->getQueryBuilder();
270242
$query->select('*')->from($this->externalShareTable)
271-
->where($query->expr()->eq('user', $query->createNamedParameter($share->getShareOwner())))
272-
->andWhere($query->expr()->eq('mountpoint', $query->createNamedParameter($share->getTarget())));
243+
->where($query->expr()->eq('user', $query->createNamedParameter($owner)))
244+
->andWhere($query->expr()->eq('mountpoint', $query->createNamedParameter($target)));
273245
$qResult = $query->executeQuery();
274246
$result = $qResult->fetchAll();
275247
$qResult->closeCursor();
@@ -478,7 +450,7 @@ public function delete(IShare $share) {
478450

479451
// only remove the share when all messages are send to not lose information
480452
// about the share to early
481-
$this->removeShareFromTable($share);
453+
$this->removeShareFromTable((int)$share->getId());
482454
}
483455

484456
/**
@@ -509,20 +481,9 @@ protected function revokeShare($share, $isOwner) {
509481
}
510482

511483
/**
512-
* remove share from table
513-
*
514-
* @param IShare $share
515-
*/
516-
public function removeShareFromTable(IShare $share) {
517-
$this->removeShareFromTableById($share->getId());
518-
}
519-
520-
/**
521-
* remove share from table
522-
*
523-
* @param string $shareId
484+
* Remove share from table.
524485
*/
525-
private function removeShareFromTableById($shareId) {
486+
public function removeShareFromTable(int $shareId): void {
526487
$qb = $this->dbConnection->getQueryBuilder();
527488
$qb->delete('share')
528489
->where($qb->expr()->eq('id', $qb->createNamedParameter($shareId)))

‎apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -409,8 +409,8 @@ protected function shareDeclined($id, array $notification) {
409409
* @param IShare $share
410410
* @throws ShareNotFound
411411
*/
412-
protected function executeDeclineShare(IShare $share) {
413-
$this->federatedShareProvider->removeShareFromTable($share);
412+
protected function executeDeclineShare(IShare $share): void {
413+
$this->federatedShareProvider->removeShareFromTable((int)$share->getId());
414414

415415
try {
416416
$fileId = (int)$share->getNode()->getId();
@@ -447,7 +447,7 @@ private function undoReshare($id, array $notification) {
447447
$share = $this->federatedShareProvider->getShareById($id);
448448

449449
$this->verifyShare($share, $token);
450-
$this->federatedShareProvider->removeShareFromTable($share);
450+
$this->federatedShareProvider->removeShareFromTable((int)$share->getId());
451451
return [];
452452
}
453453

‎apps/federatedfilesharing/tests/FederatedShareProviderTest.php‎

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,8 @@ public function testCreate($expirationDate, $expectedDataDate): void {
154154
->setPermissions(19)
155155
->setShareType(IShare::TYPE_REMOTE)
156156
->setExpirationDate($expirationDate)
157-
->setNode($node);
157+
->setNode($node)
158+
->setTarget('');
158159

159160
$this->tokenHandler->method('generateToken')->willReturn('token');
160161

@@ -235,7 +236,8 @@ public function testCreateCouldNotFindServer(): void {
235236
->setShareOwner('shareOwner')
236237
->setPermissions(19)
237238
->setShareType(IShare::TYPE_REMOTE)
238-
->setNode($node);
239+
->setNode($node)
240+
->setTarget('');
239241

240242
$this->tokenHandler->method('generateToken')->willReturn('token');
241243

@@ -296,7 +298,8 @@ public function testCreateException(): void {
296298
->setShareOwner('shareOwner')
297299
->setPermissions(19)
298300
->setShareType(IShare::TYPE_REMOTE)
299-
->setNode($node);
301+
->setNode($node)
302+
->setTarget('');
300303

301304
$this->tokenHandler->method('generateToken')->willReturn('token');
302305

@@ -404,7 +407,8 @@ public function testCreateAlreadyShared(): void {
404407
->setShareOwner('shareOwner')
405408
->setPermissions(19)
406409
->setShareType(IShare::TYPE_REMOTE)
407-
->setNode($node);
410+
->setNode($node)
411+
->setTarget('');
408412

409413
$this->tokenHandler->method('generateToken')->willReturn('token');
410414

@@ -476,7 +480,8 @@ public function testUpdate($owner, $sharedBy, $expirationDate): void {
476480
->setPermissions(19)
477481
->setShareType(IShare::TYPE_REMOTE)
478482
->setExpirationDate(new \DateTime('2019-02-01T01:02:03'))
479-
->setNode($node);
483+
->setNode($node)
484+
->setTarget('');
480485

481486
$this->tokenHandler->method('generateToken')->willReturn('token');
482487
$this->addressHandler->expects($this->any())->method('generateRemoteURL')
@@ -553,7 +558,8 @@ public function testGetSharedBy(): void {
553558
->setShareOwner('shareOwner')
554559
->setPermissions(19)
555560
->setShareType(IShare::TYPE_REMOTE)
556-
->setNode($node);
561+
->setNode($node)
562+
->setTarget('');
557563
$this->provider->create($share);
558564

559565
$share2 = $this->shareManager->newShare();
@@ -562,7 +568,8 @@ public function testGetSharedBy(): void {
562568
->setShareOwner('shareOwner')
563569
->setPermissions(19)
564570
->setShareType(IShare::TYPE_REMOTE)
565-
->setNode($node);
571+
->setNode($node)
572+
->setTarget('');
566573
$this->provider->create($share2);
567574

568575
$shares = $this->provider->getSharesBy('sharedBy', IShare::TYPE_REMOTE, null, false, -1, 0);
@@ -597,7 +604,8 @@ public function testGetSharedByWithNode(): void {
597604
->setShareOwner('shareOwner')
598605
->setPermissions(19)
599606
->setShareType(IShare::TYPE_REMOTE)
600-
->setNode($node);
607+
->setNode($node)
608+
->setTarget('');
601609
$this->provider->create($share);
602610

603611
$node2 = $this->getMockBuilder(File::class)->getMock();
@@ -610,7 +618,8 @@ public function testGetSharedByWithNode(): void {
610618
->setShareOwner('shareOwner')
611619
->setPermissions(19)
612620
->setShareType(IShare::TYPE_REMOTE)
613-
->setNode($node2);
621+
->setNode($node2)
622+
->setTarget('');
614623
$this->provider->create($share2);
615624

616625
$shares = $this->provider->getSharesBy('sharedBy', IShare::TYPE_REMOTE, $node2, false, -1, 0);
@@ -644,7 +653,8 @@ public function testGetSharedByWithReshares(): void {
644653
->setShareOwner('shareOwner')
645654
->setPermissions(19)
646655
->setShareType(IShare::TYPE_REMOTE)
647-
->setNode($node);
656+
->setNode($node)
657+
->setTarget('');
648658
$this->provider->create($share);
649659

650660
$share2 = $this->shareManager->newShare();
@@ -653,7 +663,8 @@ public function testGetSharedByWithReshares(): void {
653663
->setShareOwner('shareOwner')
654664
->setPermissions(19)
655665
->setShareType(IShare::TYPE_REMOTE)
656-
->setNode($node);
666+
->setNode($node)
667+
->setTarget('');
657668
$this->provider->create($share2);
658669

659670
$shares = $this->provider->getSharesBy('shareOwner', IShare::TYPE_REMOTE, null, true, -1, 0);
@@ -694,7 +705,8 @@ public function testGetSharedByWithLimit(): void {
694705
->setShareOwner('shareOwner')
695706
->setPermissions(19)
696707
->setShareType(IShare::TYPE_REMOTE)
697-
->setNode($node);
708+
->setNode($node)
709+
->setTarget('');
698710
$this->provider->create($share);
699711

700712
$share2 = $this->shareManager->newShare();
@@ -703,7 +715,8 @@ public function testGetSharedByWithLimit(): void {
703715
->setShareOwner('shareOwner')
704716
->setPermissions(19)
705717
->setShareType(IShare::TYPE_REMOTE)
706-
->setNode($node);
718+
->setNode($node)
719+
->setTarget('');
707720
$this->provider->create($share2);
708721

709722
$shares = $this->provider->getSharesBy('shareOwner', IShare::TYPE_REMOTE, null, true, 1, 1);
@@ -904,7 +917,8 @@ public function testGetSharesInFolder(): void {
904917
->setShareOwner($u1->getUID())
905918
->setPermissions(Constants::PERMISSION_READ)
906919
->setShareType(IShare::TYPE_REMOTE)
907-
->setNode($file1);
920+
->setNode($file1)
921+
->setTarget('');
908922
$this->provider->create($share1);
909923

910924
$share2 = $this->shareManager->newShare();
@@ -913,7 +927,8 @@ public function testGetSharesInFolder(): void {
913927
->setShareOwner($u1->getUID())
914928
->setPermissions(Constants::PERMISSION_READ)
915929
->setShareType(IShare::TYPE_REMOTE)
916-
->setNode($file2);
930+
->setNode($file2)
931+
->setTarget('');
917932
$this->provider->create($share2);
918933

919934
$result = $this->provider->getSharesInFolder($u1->getUID(), $folder1, false);
@@ -964,7 +979,8 @@ public function testGetAccessList(): void {
964979
->setShareOwner($u1->getUID())
965980
->setPermissions(Constants::PERMISSION_READ)
966981
->setShareType(IShare::TYPE_REMOTE)
967-
->setNode($file1);
982+
->setNode($file1)
983+
->setTarget('');
968984
$this->provider->create($share1);
969985

970986
$share2 = $this->shareManager->newShare();
@@ -973,7 +989,8 @@ public function testGetAccessList(): void {
973989
->setShareOwner($u1->getUID())
974990
->setPermissions(Constants::PERMISSION_READ)
975991
->setShareType(IShare::TYPE_REMOTE)
976-
->setNode($file1);
992+
->setNode($file1)
993+
->setTarget('');
977994
$this->provider->create($share2);
978995

979996
$result = $this->provider->getAccessList([$file1], true);

0 commit comments

Comments
 (0)