upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-03-12 13:44:53 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 14

Fix remote share deletion when deleting user

Browse source 
When deleting a user, we should only delete the direct remote user
shares or the remote group based subshares.

Signed-off-by: Vincent Petry <vincent@nextcloud.com>
This commit is contained in:
Vincent Petry 2021-07-27 14:32:51 +02:00
parent a22ea02bea
commit f60ea8a260
No known key found for this signature in database
GPG key ID: E055D6A4D513575C
2 changed files with 202 additions and 52 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -675,38 +675,69 @@ class Manager {
$getShare = $this->connection->prepare('
SELECT `id`, `remote`, `share_type`, `share_token`, `remote_id`
FROM `*PREFIX*share_external`
WHERE `user` = ?');
$result = $getShare->execute([$uid]);
WHERE `user` = ?
AND `share_type` = ?');
$result = $getShare->execute([$uid, IShare::TYPE_USER]);
$shares = $result->fetchAll();
$result->closeCursor();
$deletedGroupShares = [];
foreach ($shares as $share) {
if ((int)$share['share_type'] === IShare::TYPE_GROUP) {
$deletedGroupShares[] = $share['id'];
} else {
$this->sendFeedbackToRemote($share['remote'], $share['share_token'], $share['remote_id'], 'decline');
}
$this->sendFeedbackToRemote($share['remote'], $share['share_token'], $share['remote_id'], 'decline');
}
$query = $this->connection->prepare('
DELETE FROM `*PREFIX*share_external`
WHERE `user` = ?
');
$deleteResult = $query->execute([$uid]);
$deleteResult->closeCursor();
$qb = $this->connection->getQueryBuilder();
$qb->delete('share_external')
// user field can specify a user or a group
->where($qb->expr()->eq('user', $qb->createNamedParameter($uid)))
->andWhere(
$qb->expr()->orX(
// delete direct shares
$qb->expr()->eq('share_type', $qb->expr()->literal(IShare::TYPE_USER)),
// delete sub-shares of group shares for that user
$qb->expr()->andX(
$qb->expr()->eq('share_type', $qb->expr()->literal(IShare::TYPE_GROUP)),
$qb->expr()->neq('parent', $qb->expr()->literal(-1)),
)
)
);
$qb->execute();
} catch (\Doctrine\DBAL\Exception $ex) {
$this->logger->emergency('Could not delete user shares', ['exception' => $ex]);
return false;
}
// delete sub-entries from deleted parents
foreach ($deletedGroupShares as $deletedId) {
// TODO: batch this with query builder
$query = $this->connection->prepare('
DELETE FROM `*PREFIX*share_external`
WHERE `parent` = ?
');
$deleteResult = $query->execute([$deletedId]);
$deleteResult->closeCursor();
return true;
}
public function removeGroupShares($gid): bool {
try {
$getShare = $this->connection->prepare('
SELECT `id`, `remote`, `share_type`, `share_token`, `remote_id`
FROM `*PREFIX*share_external`
WHERE `user` = ?
AND `share_type` = ?');
$result = $getShare->execute([$gid, IShare::TYPE_GROUP]);
$shares = $result->fetchAll();
$result->closeCursor();
$deletedGroupShares = [];
$qb = $this->connection->getQueryBuilder();
// delete group share entry and matching sub-entries
$qb->delete('share_external')
->where(
$qb->expr()->orX(
$qb->expr()->eq('id', $qb->createParameter('share_id')),
$qb->expr()->eq('parent', $qb->createParameter('share_parent_id'))
)
);
foreach ($shares as $share) {
$qb->setParameter('share_id', $share['id']);
$qb->setParameter('share_parent_id', $share['id']);
$qb->execute();
}
} catch (\Doctrine\DBAL\Exception $ex) {
$this->logger->emergency('Could not get shares', ['exception' => $ex]);
$this->logger->emergency('Could not delete user shares', ['exception' => $ex]);
return false;
}

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

@ -82,6 +82,9 @@ class ManagerTest extends TestCase {
/** @var \PHPUnit\Framework\MockObject\MockObject|IUserManager */
private $userManager;
/** @var LoggerInterface */
private $logger;
private $uid;
/**
@ -113,27 +116,10 @@ class ManagerTest extends TestCase {
->method('search')
->willReturn([]);
$logger = $this->createMock(LoggerInterface::class);
$logger->expects($this->never())->method('emergency');
$this->logger = $this->createMock(LoggerInterface::class);
$this->logger->expects($this->never())->method('emergency');
$this->manager = $this->getMockBuilder(Manager::class)
->setConstructorArgs(
[
\OC::$server->getDatabaseConnection(),
$this->mountManager,
new StorageFactory(),
$this->clientService,
\OC::$server->getNotificationManager(),
\OC::$server->query(\OCP\OCS\IDiscoveryService::class),
$this->cloudFederationProviderManager,
$this->cloudFederationFactory,
$this->groupManager,
$this->userManager,
$this->uid,
$this->eventDispatcher,
$logger,
]
)->setMethods(['tryOCMEndPoint'])->getMock();
$this->manager = $this->createManagerForUser($this->uid);
$this->testMountProvider = new MountProvider(\OC::$server->getDatabaseConnection(), function () {
return $this->manager;
@ -165,6 +151,27 @@ class ManagerTest extends TestCase {
parent::tearDown();
}
private function createManagerForUser($userId) {
return $this->getMockBuilder(Manager::class)
->setConstructorArgs(
[
\OC::$server->getDatabaseConnection(),
$this->mountManager,
new StorageFactory(),
$this->clientService,
\OC::$server->getNotificationManager(),
\OC::$server->query(\OCP\OCS\IDiscoveryService::class),
$this->cloudFederationProviderManager,
$this->cloudFederationFactory,
$this->groupManager,
$this->userManager,
$userId,
$this->eventDispatcher,
$this->logger,
]
)->setMethods(['tryOCMEndPoint'])->getMock();
}
private function setupMounts() {
$this->mountManager->clear();
$mounts = $this->testMountProvider->getMountsForUser($this->user, new StorageFactory());
@ -348,7 +355,7 @@ class ManagerTest extends TestCase {
if ($isGroup) {
// no http requests here
$this->manager->removeUserShares('group1');
$this->manager->removeGroupShares('group1');
} else {
$client1 = $this->getMockBuilder('OCP\Http\Client\IClient')
->disableOriginalConstructor()->getMock();
@ -416,7 +423,24 @@ class ManagerTest extends TestCase {
$this->assertNotMount($tempMount);
}
private function createTestGroupShare() {
private function createTestUserShare($userId = 'user1') {
$shareData = [
'remote' => 'http://localhost',
'token' => 'token1',
'password' => '',
'name' => '/SharedFolder',
'owner' => 'foobar',
'shareType' => IShare::TYPE_USER,
'accepted' => false,
'user' => $userId,
'remoteId' => '2342'
];
$this->assertSame(null, call_user_func_array([$this->manager, 'addShare'], $shareData));
return $shareData;
}
private function createTestGroupShare($groupId = 'group1') {
$shareData = [
'remote' => 'http://localhost',
'token' => 'token1',
@ -425,17 +449,20 @@ class ManagerTest extends TestCase {
'owner' => 'foobar',
'shareType' => IShare::TYPE_GROUP,
'accepted' => false,
'user' => 'group1',
'user' => $groupId,
'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];
foreach ($allShares as $share) {
if ($share['user'] === $groupId) {
// this will hold the main group entry
$groupShare = $share;
break;
}
}
return [$shareData, $groupShare];
}
@ -461,8 +488,9 @@ class ManagerTest extends TestCase {
// this will return sub-entries
$openShares = $this->manager->getOpenShares();
$this->assertCount(1, $openShares);
// accept through sub-share
// accept through group share
$this->assertTrue($this->manager->acceptShare($groupShare['id']));
$this->verifyAcceptedGroupShare($shareData, '/SharedFolder');
@ -482,6 +510,7 @@ class ManagerTest extends TestCase {
// this will return sub-entries
$openShares = $this->manager->getOpenShares();
$this->assertCount(1, $openShares);
// accept through sub-share
$this->assertTrue($this->manager->acceptShare($openShares[0]['id']));
@ -583,6 +612,96 @@ class ManagerTest extends TestCase {
$this->verifyAcceptedGroupShare($shareData);
}
public function testDeleteUserShares() {
// user 1 shares
$shareData = $this->createTestUserShare($this->uid);
[$shareData, $groupShare] = $this->createTestGroupShare();
$shares = $this->manager->getOpenShares();
$this->assertCount(2, $shares);
$this->assertTrue($this->manager->acceptShare($groupShare['id']));
// user 2 shares
$manager2 = $this->createManagerForUser('user2');
$shareData2 = [
'remote' => 'http://localhost',
'token' => 'token1',
'password' => '',
'name' => '/SharedFolder',
'owner' => 'foobar',
'shareType' => IShare::TYPE_USER,
'accepted' => false,
'user' => 'user2',
'remoteId' => '2342'
];
$this->assertSame(null, call_user_func_array([$manager2, 'addShare'], $shareData2));
$user2Shares = $manager2->getOpenShares();
$this->assertCount(2, $user2Shares);
$this->manager->expects($this->at(0))->method('tryOCMEndPoint')->with('http://localhost', 'token1', '2342', 'decline')->willReturn([]);
$this->manager->removeUserShares($this->uid);
$user1Shares = $this->manager->getOpenShares();
// user share is gone, group is still there
$this->assertCount(1, $user1Shares);
$this->assertEquals($user1Shares[0]['share_type'], IShare::TYPE_GROUP);
// user 2 shares untouched
$user2Shares = $manager2->getOpenShares();
$this->assertCount(2, $user2Shares);
$this->assertEquals($user2Shares[0]['share_type'], IShare::TYPE_GROUP);
$this->assertEquals($user2Shares[0]['user'], 'group1');
$this->assertEquals($user2Shares[1]['share_type'], IShare::TYPE_USER);
$this->assertEquals($user2Shares[1]['user'], 'user2');
}
public function testDeleteGroupShares() {
$shareData = $this->createTestUserShare($this->uid);
[$shareData, $groupShare] = $this->createTestGroupShare();
$shares = $this->manager->getOpenShares();
$this->assertCount(2, $shares);
$this->assertTrue($this->manager->acceptShare($groupShare['id']));
// user 2 shares
$manager2 = $this->createManagerForUser('user2');
$shareData2 = [
'remote' => 'http://localhost',
'token' => 'token1',
'password' => '',
'name' => '/SharedFolder',
'owner' => 'foobar',
'shareType' => IShare::TYPE_USER,
'accepted' => false,
'user' => 'user2',
'remoteId' => '2342'
];
$this->assertSame(null, call_user_func_array([$manager2, 'addShare'], $shareData2));
$user2Shares = $manager2->getOpenShares();
$this->assertCount(2, $user2Shares);
$this->manager->expects($this->never())->method('tryOCMEndPoint');
$this->manager->removeGroupShares('group1');
$user1Shares = $this->manager->getOpenShares();
// user share is gone, group is still there
$this->assertCount(1, $user1Shares);
$this->assertEquals($user1Shares[0]['share_type'], IShare::TYPE_USER);
// user 2 shares untouched
$user2Shares = $manager2->getOpenShares();
$this->assertCount(1, $user2Shares);
$this->assertEquals($user2Shares[0]['share_type'], IShare::TYPE_USER);
$this->assertEquals($user2Shares[0]['user'], 'user2');
}
/**
* @param array $expected
* @param array $actual