Skip to content

Commit 9dc25bc

Browse files
provokateurinprovokateurin
committed
fix(federatedfilesharing): Do not set the share id for an existing share
Signed-off-by: provokateurin <kate@provokateurin.de>
1 parent 6ad26d9 commit 9dc25bc

3 files changed

Lines changed: 64 additions & 56 deletions

File tree

‎apps/federatedfilesharing/lib/FederatedShareProvider.php‎

Lines changed: 27 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -130,30 +130,30 @@ 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+
$shareType,
150+
);
151+
// remote share was create successfully if we get a valid token as return
152+
if (is_string($token) && $token !== '') {
153153
$this->updateSuccessfulReshare($shareId, $token);
154154
$this->storeRemoteId($shareId, $remoteId);
155155
} else {
156-
$this->removeShareFromTable($share);
156+
$this->removeShareFromTable($shareId);
157157
$message_t = $this->l->t('File is already shared with %s', [$shareWith]);
158158
throw new \Exception($message_t);
159159
}
@@ -220,7 +220,7 @@ protected function createFederatedShare(IShare $share) {
220220
}
221221

222222
if ($failure) {
223-
$this->removeShareFromTableById($shareId);
223+
$this->removeShareFromTable($shareId);
224224
$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.',
225225
[$share->getNode()->getName(), $share->getSharedWith()]);
226226
throw new \Exception($message_t);
@@ -230,6 +230,7 @@ protected function createFederatedShare(IShare $share) {
230230
}
231231

232232
/**
233+
<<<<<<< HEAD
233234
* @param string $shareWith
234235
* @param IShare $share
235236
* @param string $shareId internal share Id
@@ -256,18 +257,19 @@ protected function askOwnerToReShare($shareWith, IShare $share, $shareId) {
256257
}
257258

258259
/**
260+
=======
261+
>>>>>>> 045ad43237c (fix(federatedfilesharing): Do not set the share id for an existing share)
259262
* get federated share from the share_external table but exclude mounted link shares
260263
*
261-
* @param IShare $share
262264
* @return array
263265
* @throws ShareNotFound
264266
*/
265-
protected function getShareFromExternalShareTable(IShare $share) {
267+
protected function getShareFromExternalShareTable(string $owner, string $target) {
266268
$query = $this->dbConnection->getQueryBuilder();
267269
$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();
270+
->where($query->expr()->eq('user', $query->createNamedParameter($owner)))
271+
->andWhere($query->expr()->eq('mountpoint', $query->createNamedParameter($target)));
272+
$qResult = $query->executeQuery();
271273
$result = $qResult->fetchAll();
272274
$qResult->closeCursor();
273275

@@ -475,7 +477,7 @@ public function delete(IShare $share) {
475477

476478
// only remove the share when all messages are send to not lose information
477479
// about the share to early
478-
$this->removeShareFromTable($share);
480+
$this->removeShareFromTable($share->getId());
479481
}
480482

481483
/**
@@ -506,20 +508,9 @@ protected function revokeShare($share, $isOwner) {
506508
}
507509

508510
/**
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
511+
* Remove share from table.
521512
*/
522-
private function removeShareFromTableById($shareId) {
513+
public function removeShareFromTable(string $shareId): void {
523514
$qb = $this->dbConnection->getQueryBuilder();
524515
$qb->delete('share')
525516
->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($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($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)