upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-06-08 16:26:59 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 12

Fix remote group share API interactions

Browse source 
Accepting and declining can now be done repeatedly on both the parent
group share and sub-share with the same effects.

Added unit tests to cover these cases, and also when the same operation
is repeated.

Signed-off-by: Vincent Petry <vincent@nextcloud.com>
This commit is contained in:
Vincent Petry 2021-07-05 16:07:02 +02:00
parent dde4c9d9df
commit 701de23857
No known key found for this signature in database
GPG key ID: E055D6A4D513575C
2 changed files with 199 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

44
apps/files_sharing/lib/External/Manager.php vendored
View file

@ -238,6 +238,20 @@ class Manager {
return $share;
}
private function fetchUserShare($parentId, $uid) {
$getShare = $this->connection->prepare('
SELECT `id`, `remote`, `remote_id`, `share_token`, `name`, `owner`, `user`, `mountpoint`, `accepted`, `parent`, `share_type`, `password`, `mountpoint_hash`
FROM `*PREFIX*share_external`
WHERE `parent` = ? AND `user` = ?');
$result = $getShare->execute([$parentId, $uid]);
$share = $result->fetch();
$result->closeCursor();
if ($share !== false) {
return $share;
}
return null;
}
/**
* get share
*
@ -311,9 +325,15 @@ class Manager {
} else {
$parentId = (int)$share['parent'];
if ($parentId !== -1) {
// this is the sub-share, simply update it to re-accept
// this is the sub-share
$subshare = $share;
} else {
$subshare = $this->fetchUserShare($id, $this->uid);
}
if ($subshare !== null) {
try {
$this->updateAccepted((int)$share['id'], true);
$this->updateAccepted((int)$subshare['id'], true);
$result = true;
} catch (Exception $e) {
$this->logger->logException($e);
@ -372,13 +392,17 @@ class Manager {
$this->processNotification($id);
$result = true;
} elseif ($share && (int)$share['share_type'] === IShare::TYPE_GROUP) {
$parent = (int)$share['parent'];
// can only decline an already accepted/mounted group share,
// check if this is the sub-share entry
if ($parent !== -1) {
$parentId = (int)$share['parent'];
if ($parentId !== -1) {
// this is the sub-share
$subshare = $share;
} else {
$subshare = $this->fetchUserShare($id, $this->uid);
}
if ($subshare !== null) {
try {
// this is the sub-share, simply update it to decline
$this->updateAccepted((int)$share['id'], false);
$this->updateAccepted((int)$subshare['id'], false);
$result = true;
} catch (Exception $e) {
$this->logger->logException($e);
@ -566,6 +590,10 @@ class Manager {
public function removeShare($mountPoint): bool {
$mountPointObj = $this->mountManager->find($mountPoint);
if ($mountPointObj === null) {
$this->logger->error('Mount point to remove share not found', ['mountPoint' => $mountPoint]);
return false;
}
$id = $mountPointObj->getStorage()->getCache()->getId('');
$mountPoint = $this->stripPath($mountPoint);

166
apps/files_sharing/tests/External/ManagerTest.php vendored
View file

@ -113,6 +113,9 @@ class ManagerTest extends TestCase {
->method('search')
->willReturn([]);
$logger = $this->createMock(ILogger::class);
$logger->expects($this->never())->method('logException');
$this->manager = $this->getMockBuilder(Manager::class)
->setConstructorArgs(
[
@ -128,7 +131,7 @@ class ManagerTest extends TestCase {
$this->userManager,
$this->uid,
$this->eventDispatcher,
$this->createMock(ILogger::class),
$logger,
]
)->setMethods(['tryOCMEndPoint'])->getMock();
@ -155,6 +158,7 @@ class ManagerTest extends TestCase {
}
private function setupMounts() {
$this->mountManager->clear();
$mounts = $this->testMountProvider->getMountsForUser($this->user, new StorageFactory());
foreach ($mounts as $mount) {
$this->mountManager->addMount($mount);
@ -247,7 +251,7 @@ class ManagerTest extends TestCase {
}
// Accept the first share
$this->manager->acceptShare($openShares[0]['id']);
$this->assertTrue($this->manager->acceptShare($openShares[0]['id']));
// Check remaining shares - Accepted
$acceptedShares = self::invokePrivate($this->manager, 'getShares', [true]);
@ -303,7 +307,7 @@ class ManagerTest extends TestCase {
}
// Decline the third share
$this->manager->declineShare($openShares[1]['id']);
$this->assertTrue($this->manager->declineShare($openShares[1]['id']));
$this->setupMounts();
$this->assertMount($shareData1['name']);
@ -379,6 +383,162 @@ class ManagerTest extends TestCase {
$this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}-1');
}
private function verifyAcceptedGroupShare($shareData) {
$openShares = $this->manager->getOpenShares();
$this->assertCount(0, $openShares);
$acceptedShares = self::invokePrivate($this->manager, 'getShares', [true]);
$this->assertCount(1, $acceptedShares);
$shareData['accepted'] = true;
$this->assertExternalShareEntry($shareData, $acceptedShares[0], 0, $shareData['name'], $this->uid);
$this->setupMounts();
$this->assertMount($shareData['name']);
}
private function verifyDeclinedGroupShare($shareData, $tempMount = null) {
if ($tempMount === null) {
$tempMount = '{{TemporaryMountPointName#/SharedFolder}}';
}
$openShares = $this->manager->getOpenShares();
$this->assertCount(1, $openShares);
$acceptedShares = self::invokePrivate($this->manager, 'getShares', [true]);
$this->assertCount(0, $acceptedShares);
$this->assertExternalShareEntry($shareData, $openShares[0], 0, $tempMount, $this->uid);
$this->setupMounts();
$this->assertNotMount($shareData['name']);
$this->assertNotMount($tempMount);
}
private function createTestGroupShare() {
$shareData = [
'remote' => 'http://localhost',
'token' => 'token1',
'password' => '',
'name' => '/SharedFolder',
'owner' => 'foobar',
'shareType' => IShare::TYPE_GROUP,
'accepted' => false,
'user' => 'group1',
'remoteId' => '2342'
];
$this->assertSame(null, call_user_func_array([$this->manager, 'addShare'], $shareData));
$allShares = self::invokePrivate($this->manager, 'getShares', [null]);
$this->assertCount(1, $allShares);
// this will hold the main group entry
$groupShare = $allShares[0];
return [$shareData, $groupShare];
}
public function testAcceptOriginalGroupShare() {
[$shareData, $groupShare] = $this->createTestGroupShare();
$this->assertTrue($this->manager->acceptShare($groupShare['id']));
$this->verifyAcceptedGroupShare($shareData);
// a second time
$this->assertTrue($this->manager->acceptShare($groupShare['id']));
$this->verifyAcceptedGroupShare($shareData);
}
public function testAcceptGroupShareAgainThroughGroupShare() {
[$shareData, $groupShare] = $this->createTestGroupShare();
$this->assertTrue($this->manager->acceptShare($groupShare['id']));
$this->verifyAcceptedGroupShare($shareData);
// decline again, this keeps the sub-share
$this->assertTrue($this->manager->declineShare($groupShare['id']));
$this->verifyDeclinedGroupShare($shareData, '/SharedFolder');
// this will return sub-entries
$openShares = $this->manager->getOpenShares();
// accept through sub-share
$this->assertTrue($this->manager->acceptShare($groupShare['id']));
$this->verifyAcceptedGroupShare($shareData, '/SharedFolder');
// accept a second time
$this->assertTrue($this->manager->acceptShare($groupShare['id']));
$this->verifyAcceptedGroupShare($shareData, '/SharedFolder');
}
public function testAcceptGroupShareAgainThroughSubShare() {
[$shareData, $groupShare] = $this->createTestGroupShare();
$this->assertTrue($this->manager->acceptShare($groupShare['id']));
$this->verifyAcceptedGroupShare($shareData);
// decline again, this keeps the sub-share
$this->assertTrue($this->manager->declineShare($groupShare['id']));
$this->verifyDeclinedGroupShare($shareData, '/SharedFolder');
// this will return sub-entries
$openShares = $this->manager->getOpenShares();
// accept through sub-share
$this->assertTrue($this->manager->acceptShare($openShares[0]['id']));
$this->verifyAcceptedGroupShare($shareData);
// accept a second time
$this->assertTrue($this->manager->acceptShare($openShares[0]['id']));
$this->verifyAcceptedGroupShare($shareData);
}
public function testDeclineOriginalGroupShare() {
[$shareData, $groupShare] = $this->createTestGroupShare();
$this->assertTrue($this->manager->declineShare($groupShare['id']));
$this->verifyDeclinedGroupShare($shareData);
// a second time
$this->assertTrue($this->manager->declineShare($groupShare['id']));
$this->verifyDeclinedGroupShare($shareData);
}
public function testDeclineGroupShareAgainThroughGroupShare() {
[$shareData, $groupShare] = $this->createTestGroupShare();
$this->assertTrue($this->manager->acceptShare($groupShare['id']));
$this->verifyAcceptedGroupShare($shareData);
// decline again, this keeps the sub-share
$this->assertTrue($this->manager->declineShare($groupShare['id']));
$this->verifyDeclinedGroupShare($shareData, '/SharedFolder');
// a second time
$this->assertTrue($this->manager->declineShare($groupShare['id']));
$this->verifyDeclinedGroupShare($shareData, '/SharedFolder');
}
public function testDeclineGroupShareAgainThroughSubshare() {
[$shareData, $groupShare] = $this->createTestGroupShare();
$this->assertTrue($this->manager->acceptShare($groupShare['id']));
$this->verifyAcceptedGroupShare($shareData);
// this will return sub-entries
$allShares = self::invokePrivate($this->manager, 'getShares', [null]);
$this->assertCount(1, $allShares);
// decline again through sub-share
$this->assertTrue($this->manager->declineShare($allShares[0]['id']));
$this->verifyDeclinedGroupShare($shareData, '/SharedFolder');
// a second time
$this->assertTrue($this->manager->declineShare($allShares[0]['id']));
$this->verifyDeclinedGroupShare($shareData, '/SharedFolder');
}
public function testDeclineGroupShareAgainThroughMountPoint() {
[$shareData, $groupShare] = $this->createTestGroupShare();
$this->assertTrue($this->manager->acceptShare($groupShare['id']));
$this->verifyAcceptedGroupShare($shareData);
// decline through mount point name
$this->assertTrue($this->manager->removeShare($this->uid . '/files/' . $shareData['name']));
$this->verifyDeclinedGroupShare($shareData, '/SharedFolder');
// second time must fail as the mount point is gone
$this->assertFalse($this->manager->removeShare($this->uid . '/files/' . $shareData['name']));
}
/**
* @param array $expected
* @param array $actual