Skip to content

Commit f75aad1

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 86e1e7c commit f75aad1

3 files changed

Lines changed: 64 additions & 57 deletions

File tree

‎apps/federatedfilesharing/lib/FederatedShareProvider.php‎

Lines changed: 27 additions & 37 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);
@@ -230,6 +229,7 @@ protected function createFederatedShare(IShare $share) {
230229
}
231230

232231
/**
232+
<<<<<<< HEAD
233233
* @param string $shareWith
234234
* @param IShare $share
235235
* @param string $shareId internal share Id
@@ -256,18 +256,19 @@ protected function askOwnerToReShare($shareWith, IShare $share, $shareId) {
256256
}
257257

258258
/**
259+
=======
260+
>>>>>>> 045ad43237c (fix(federatedfilesharing): Do not set the share id for an existing share)
259261
* get federated share from the share_external table but exclude mounted link shares
260262
*
261-
* @param IShare $share
262263
* @return array
263264
* @throws ShareNotFound
264265
*/
265-
protected function getShareFromExternalShareTable(IShare $share) {
266+
protected function getShareFromExternalShareTable(string $owner, string $target) {
266267
$query = $this->dbConnection->getQueryBuilder();
267268
$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();
269+
->where($query->expr()->eq('user', $query->createNamedParameter($owner)))
270+
->andWhere($query->expr()->eq('mountpoint', $query->createNamedParameter($target)));
271+
$qResult = $query->executeQuery();
271272
$result = $qResult->fetchAll();
272273
$qResult->closeCursor();
273274

@@ -289,7 +290,7 @@ protected function getShareFromExternalShareTable(IShare $share) {
289290
* @param int $permissions
290291
* @param string $token
291292
* @param int $shareType
292-
* @param \DateTime $expirationDate
293+
* @param ?\DateTime $expirationDate
293294
* @return int
294295
*/
295296
private function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $uidOwner, $permissions, $token, $shareType, $expirationDate) {
@@ -475,7 +476,7 @@ public function delete(IShare $share) {
475476

476477
// only remove the share when all messages are send to not lose information
477478
// about the share to early
478-
$this->removeShareFromTable($share);
479+
$this->removeShareFromTable($share->getId());
479480
}
480481

481482
/**
@@ -506,20 +507,9 @@ protected function revokeShare($share, $isOwner) {
506507
}
507508

508509
/**
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
510+
* Remove share from table.
521511
*/
522-
private function removeShareFromTableById($shareId) {
512+
public function removeShareFromTable(int $shareId): void {
523513
$qb = $this->dbConnection->getQueryBuilder();
524514
$qb->delete('share')
525515
->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)