Skip to content

Commit 08d012c

Browse files
sorbaughsorbaugh
authored
Merge pull request #58992 from nextcloud/backport/58689/stable30
[stable30] Fix federated reshares
2 parents 6a74a82 + eb9f88e commit 08d012c

6 files changed

Lines changed: 64 additions & 88 deletions

File tree

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@
5858
}
5959

6060
$share = $authBackend->getShare();
61-
$owner = $share->getShareOwner();
6261
$isReadable = $share->getPermissions() & \OCP\Constants::PERMISSION_READ;
6362
$fileId = $share->getNodeId();
6463

@@ -73,7 +72,7 @@
7372
\OC\Files\Filesystem::logWarningWhenAddingStorageWrapper($previousLog);
7473

7574
$rootFolder = \OCP\Server::get(\OCP\Files\IRootFolder::class);
76-
$userFolder = $rootFolder->getUserFolder($owner);
75+
$userFolder = $rootFolder->getUserFolder($share->getSharedBy());
7776
$node = $userFolder->getFirstNodeById($fileId);
7877
if (!$node) {
7978
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
@@ -85,7 +85,6 @@
8585
}
8686

8787
$share = $authBackend->getShare();
88-
$owner = $share->getShareOwner();
8988
$isReadable = $share->getPermissions() & \OCP\Constants::PERMISSION_READ;
9089
$fileId = $share->getNodeId();
9190

@@ -113,7 +112,7 @@
113112
Filesystem::logWarningWhenAddingStorageWrapper($previousLog);
114113

115114
$rootFolder = \OCP\Server::get(\OCP\Files\IRootFolder::class);
116-
$userFolder = $rootFolder->getUserFolder($owner);
115+
$userFolder = $rootFolder->getUserFolder($share->getSharedBy());
117116
$node = $userFolder->getFirstNodeById($fileId);
118117
if (!$node) {
119118
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 his 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: 24 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -130,30 +130,29 @@ public function create(IShare $share) {
130130
$share->setSharedWith($cloudId->getId());
131131

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

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

222221
if ($failure) {
223-
$this->removeShareFromTableById($shareId);
222+
$this->removeShareFromTable($shareId);
224223
$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.',
225224
[$share->getNode()->getName(), $share->getSharedWith()]);
226225
throw new \Exception($message_t);
@@ -229,45 +228,18 @@ protected function createFederatedShare(IShare $share) {
229228
return $shareId;
230229
}
231230

232-
/**
233-
* @param string $shareWith
234-
* @param IShare $share
235-
* @param string $shareId internal share Id
236-
* @return array
237-
* @throws \Exception
238-
*/
239-
protected function askOwnerToReShare($shareWith, IShare $share, $shareId) {
240-
$remoteShare = $this->getShareFromExternalShareTable($share);
241-
$token = $remoteShare['share_token'];
242-
$remoteId = $remoteShare['remote_id'];
243-
$remote = $remoteShare['remote'];
244-
245-
[$token, $remoteId] = $this->notifications->requestReShare(
246-
$token,
247-
$remoteId,
248-
$shareId,
249-
$remote,
250-
$shareWith,
251-
$share->getPermissions(),
252-
$share->getNode()->getName()
253-
);
254-
255-
return [$token, $remoteId];
256-
}
257-
258231
/**
259232
* get federated share from the share_external table but exclude mounted link shares
260233
*
261-
* @param IShare $share
262234
* @return array
263235
* @throws ShareNotFound
264236
*/
265-
protected function getShareFromExternalShareTable(IShare $share) {
237+
protected function getShareFromExternalShareTable(string $owner, string $target) {
266238
$query = $this->dbConnection->getQueryBuilder();
267239
$query->select('*')->from($this->externalShareTable)
268-
->where($query->expr()->eq('user', $query->createNamedParameter($share->getShareOwner())))
269-
->andWhere($query->expr()->eq('mountpoint', $query->createNamedParameter($share->getTarget())));
270-
$qResult = $query->execute();
240+
->where($query->expr()->eq('user', $query->createNamedParameter($owner)))
241+
->andWhere($query->expr()->eq('mountpoint', $query->createNamedParameter($target)));
242+
$qResult = $query->executeQuery();
271243
$result = $qResult->fetchAll();
272244
$qResult->closeCursor();
273245

@@ -289,7 +261,7 @@ protected function getShareFromExternalShareTable(IShare $share) {
289261
* @param int $permissions
290262
* @param string $token
291263
* @param int $shareType
292-
* @param \DateTime $expirationDate
264+
* @param ?\DateTime $expirationDate
293265
* @return int
294266
*/
295267
private function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $uidOwner, $permissions, $token, $shareType, $expirationDate) {
@@ -475,7 +447,7 @@ public function delete(IShare $share) {
475447

476448
// only remove the share when all messages are send to not lose information
477449
// about the share to early
478-
$this->removeShareFromTable($share);
450+
$this->removeShareFromTable((int) $share->getId());
479451
}
480452

481453
/**
@@ -506,20 +478,9 @@ protected function revokeShare($share, $isOwner) {
506478
}
507479

508480
/**
509-
* remove share from table
510-
*
511-
* @param IShare $share
512-
*/
513-
public function removeShareFromTable(IShare $share) {
514-
$this->removeShareFromTableById($share->getId());
515-
}
516-
517-
/**
518-
* remove share from table
519-
*
520-
* @param string $shareId
481+
* Remove share from table.
521482
*/
522-
private function removeShareFromTableById($shareId) {
483+
public function removeShareFromTable(int $shareId): void {
523484
$qb = $this->dbConnection->getQueryBuilder();
524485
$qb->delete('share')
525486
->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
@@ -382,8 +382,8 @@ protected function shareDeclined($id, array $notification) {
382382
* @param IShare $share
383383
* @throws ShareNotFound
384384
*/
385-
protected function executeDeclineShare(IShare $share) {
386-
$this->federatedShareProvider->removeShareFromTable($share);
385+
protected function executeDeclineShare(IShare $share): void {
386+
$this->federatedShareProvider->removeShareFromTable((int) $share->getId());
387387

388388
try {
389389
$fileId = (int) $share->getNode()->getId();
@@ -420,7 +420,7 @@ private function undoReshare($id, array $notification) {
420420
$share = $this->federatedShareProvider->getShareById($id);
421421

422422
$this->verifyShare($share, $token);
423-
$this->federatedShareProvider->removeShareFromTable($share);
423+
$this->federatedShareProvider->removeShareFromTable((int) $share->getId());
424424
return [];
425425
}
426426

‎apps/federatedfilesharing/tests/FederatedShareProviderTest.php‎

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

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

@@ -234,7 +235,8 @@ public function testCreateCouldNotFindServer() {
234235
->setShareOwner('shareOwner')
235236
->setPermissions(19)
236237
->setShareType(IShare::TYPE_REMOTE)
237-
->setNode($node);
238+
->setNode($node)
239+
->setTarget('');
238240

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

@@ -295,7 +297,8 @@ public function testCreateException() {
295297
->setShareOwner('shareOwner')
296298
->setPermissions(19)
297299
->setShareType(IShare::TYPE_REMOTE)
298-
->setNode($node);
300+
->setNode($node)
301+
->setTarget('');
299302

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

@@ -403,7 +406,8 @@ public function testCreateAlreadyShared() {
403406
->setShareOwner('shareOwner')
404407
->setPermissions(19)
405408
->setShareType(IShare::TYPE_REMOTE)
406-
->setNode($node);
409+
->setNode($node)
410+
->setTarget('');
407411

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

@@ -475,7 +479,8 @@ public function testUpdate($owner, $sharedBy, $expirationDate) {
475479
->setPermissions(19)
476480
->setShareType(IShare::TYPE_REMOTE)
477481
->setExpirationDate(new \DateTime('2019-02-01T01:02:03'))
478-
->setNode($node);
482+
->setNode($node)
483+
->setTarget('');
479484

480485
$this->tokenHandler->method('generateToken')->willReturn('token');
481486
$this->addressHandler->expects($this->any())->method('generateRemoteURL')
@@ -552,7 +557,8 @@ public function testGetSharedBy() {
552557
->setShareOwner('shareOwner')
553558
->setPermissions(19)
554559
->setShareType(IShare::TYPE_REMOTE)
555-
->setNode($node);
560+
->setNode($node)
561+
->setTarget('');
556562
$this->provider->create($share);
557563

558564
$share2 = $this->shareManager->newShare();
@@ -561,7 +567,8 @@ public function testGetSharedBy() {
561567
->setShareOwner('shareOwner')
562568
->setPermissions(19)
563569
->setShareType(IShare::TYPE_REMOTE)
564-
->setNode($node);
570+
->setNode($node)
571+
->setTarget('');
565572
$this->provider->create($share2);
566573

567574
$shares = $this->provider->getSharesBy('sharedBy', IShare::TYPE_REMOTE, null, false, -1, 0);
@@ -596,7 +603,8 @@ public function testGetSharedByWithNode() {
596603
->setShareOwner('shareOwner')
597604
->setPermissions(19)
598605
->setShareType(IShare::TYPE_REMOTE)
599-
->setNode($node);
606+
->setNode($node)
607+
->setTarget('');
600608
$this->provider->create($share);
601609

602610
$node2 = $this->getMockBuilder(File::class)->getMock();
@@ -609,7 +617,8 @@ public function testGetSharedByWithNode() {
609617
->setShareOwner('shareOwner')
610618
->setPermissions(19)
611619
->setShareType(IShare::TYPE_REMOTE)
612-
->setNode($node2);
620+
->setNode($node2)
621+
->setTarget('');
613622
$this->provider->create($share2);
614623

615624
$shares = $this->provider->getSharesBy('sharedBy', IShare::TYPE_REMOTE, $node2, false, -1, 0);
@@ -643,7 +652,8 @@ public function testGetSharedByWithReshares() {
643652
->setShareOwner('shareOwner')
644653
->setPermissions(19)
645654
->setShareType(IShare::TYPE_REMOTE)
646-
->setNode($node);
655+
->setNode($node)
656+
->setTarget('');
647657
$this->provider->create($share);
648658

649659
$share2 = $this->shareManager->newShare();
@@ -652,7 +662,8 @@ public function testGetSharedByWithReshares() {
652662
->setShareOwner('shareOwner')
653663
->setPermissions(19)
654664
->setShareType(IShare::TYPE_REMOTE)
655-
->setNode($node);
665+
->setNode($node)
666+
->setTarget('');
656667
$this->provider->create($share2);
657668

658669
$shares = $this->provider->getSharesBy('shareOwner', IShare::TYPE_REMOTE, null, true, -1, 0);
@@ -693,7 +704,8 @@ public function testGetSharedByWithLimit() {
693704
->setShareOwner('shareOwner')
694705
->setPermissions(19)
695706
->setShareType(IShare::TYPE_REMOTE)
696-
->setNode($node);
707+
->setNode($node)
708+
->setTarget('');
697709
$this->provider->create($share);
698710

699711
$share2 = $this->shareManager->newShare();
@@ -702,7 +714,8 @@ public function testGetSharedByWithLimit() {
702714
->setShareOwner('shareOwner')
703715
->setPermissions(19)
704716
->setShareType(IShare::TYPE_REMOTE)
705-
->setNode($node);
717+
->setNode($node)
718+
->setTarget('');
706719
$this->provider->create($share2);
707720

708721
$shares = $this->provider->getSharesBy('shareOwner', IShare::TYPE_REMOTE, null, true, 1, 1);
@@ -903,7 +916,8 @@ public function testGetSharesInFolder() {
903916
->setShareOwner($u1->getUID())
904917
->setPermissions(\OCP\Constants::PERMISSION_READ)
905918
->setShareType(IShare::TYPE_REMOTE)
906-
->setNode($file1);
919+
->setNode($file1)
920+
->setTarget('');
907921
$this->provider->create($share1);
908922

909923
$share2 = $this->shareManager->newShare();
@@ -912,7 +926,8 @@ public function testGetSharesInFolder() {
912926
->setShareOwner($u1->getUID())
913927
->setPermissions(\OCP\Constants::PERMISSION_READ)
914928
->setShareType(IShare::TYPE_REMOTE)
915-
->setNode($file2);
929+
->setNode($file2)
930+
->setTarget('');
916931
$this->provider->create($share2);
917932

918933
$result = $this->provider->getSharesInFolder($u1->getUID(), $folder1, false);
@@ -963,7 +978,8 @@ public function testGetAccessList() {
963978
->setShareOwner($u1->getUID())
964979
->setPermissions(\OCP\Constants::PERMISSION_READ)
965980
->setShareType(IShare::TYPE_REMOTE)
966-
->setNode($file1);
981+
->setNode($file1)
982+
->setTarget('');
967983
$this->provider->create($share1);
968984

969985
$share2 = $this->shareManager->newShare();
@@ -972,7 +988,8 @@ public function testGetAccessList() {
972988
->setShareOwner($u1->getUID())
973989
->setPermissions(\OCP\Constants::PERMISSION_READ)
974990
->setShareType(IShare::TYPE_REMOTE)
975-
->setNode($file1);
991+
->setNode($file1)
992+
->setTarget('');
976993
$this->provider->create($share2);
977994

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

0 commit comments

Comments
 (0)