mbk-lab
/
nextcloud
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Add repair step for unmerged shares (WIP)

This commit is contained in:
Vincent Petry 2016-07-12 13:38:49 +02:00 committed by Roeland Jago Douma
parent 3e8b787dd1
commit 6638e1857a
No known key found for this signature in database
GPG Key ID: 1E152838F164D13B
3 changed files with 739 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
lib/private/Repair.php
View File

@ -49,6 +49,7 @@ use OC\Repair\RepairMimeTypes;
use OC\Repair\SearchLuceneTables;
use OC\Repair\UpdateOutdatedOcsIds;
use OC\Repair\RepairInvalidShares;
use OC\Repair\RepairUnmergedShares;
use OCP\AppFramework\QueryException;
use OCP\Migration\IOutput;
use OCP\Migration\IRepairStep;
@ -140,6 +141,12 @@ class Repair implements IOutput{
new RemoveOldShares(\OC::$server->getDatabaseConnection()),
new AvatarPermissions(\OC::$server->getDatabaseConnection()),
new RemoveRootShares(\OC::$server->getDatabaseConnection(), \OC::$server->getUserManager(), \OC::$server->getLazyRootFolder()),
new RepairUnmergedShares(
\OC::$server->getConfig(),
\OC::$server->getDatabaseConnection(),
\OC::$server->getUserManager(),
\OC::$server->getGroupManager()
),
];
}

323
lib/private/Repair/RepairUnmergedShares.php Normal file
View File

@ -0,0 +1,323 @@
<?php
/**
* @author Vincent Petry <pvince81@owncloud.com>
*
* @copyright Copyright (c) 2016, ownCloud, Inc.
* @license AGPL-3.0
*
* This code is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License, version 3,
* as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License, version 3,
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/
namespace OC\Repair;
use OCP\Migration\IOutput;
use OCP\Migration\IRepairStep;
use OC\Share\Constants;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IUserManager;
use OCP\IUser;
use OCP\IGroupManager;
use OC\Share20\DefaultShareProvider;
/**
* Repairs shares for which the received folder was not properly deduplicated.
*
* An unmerged share can for example happen when sharing a folder with the same
* user through multiple ways, like several groups and also directly, additionally
* to group shares. Since 9.0.0 these would create duplicate entries "folder (2)",
* one for every share. This repair step rearranges them so they only appear as a single
* folder.
*/
class RepairUnmergedShares implements IRepairStep {
/** @var \OCP\IConfig */
protected $config;
/** @var \OCP\IDBConnection */
protected $connection;
/** @var IUserManager */
protected $userManager;
/** @var IGroupManager */
protected $groupManager;
/** @var IQueryBuilder */
private $queryGetSharesWithUsers;
/** @var IQueryBuilder */
private $queryUpdateSharePermissionsAndTarget;
/** @var IQueryBuilder */
private $queryUpdateShareInBatch;
/**
* @param \OCP\IConfig $config
* @param \OCP\IDBConnection $connection
*/
public function __construct(
IConfig $config,
IDBConnection $connection,
IUserManager $userManager,
IGroupManager $groupManager
) {
$this->connection = $connection;
$this->config = $config;
$this->userManager = $userManager;
$this->groupManager = $groupManager;
}
public function getName() {
return 'Repair unmerged shares';
}
/**
* Builds prepared queries for reuse
*/
private function buildPreparedQueries() {
/**
* Retrieve shares for a given user/group and share type
*/
$query = $this->connection->getQueryBuilder();
$query
->select('item_source', 'id', 'file_target', 'permissions', 'parent', 'share_type')
->from('share')
->where($query->expr()->eq('share_type', $query->createParameter('shareType')))
->andWhere($query->expr()->in('share_with', $query->createParameter('shareWiths')))
->andWhere($query->expr()->in('item_type', $query->createParameter('itemTypes')))
->orderBy('item_source', 'ASC')
->addOrderBy('stime', 'ASC');
$this->queryGetSharesWithUsers = $query;
/**
* Updates the file_target to the given value for all given share ids.
*
* This updates several shares in bulk which is faster than individually.
*/
$query = $this->connection->getQueryBuilder();
$query->update('share')
->set('file_target', $query->createParameter('file_target'))
->where($query->expr()->in('id', $query->createParameter('ids')));
$this->queryUpdateShareInBatch = $query;
/**
* Updates the share permissions and target path of a single share.
*/
$query = $this->connection->getQueryBuilder();
$query->update('share')
->set('permissions', $query->createParameter('permissions'))
->set('file_target', $query->createParameter('file_target'))
->where($query->expr()->eq('id', $query->createParameter('shareid')));
$this->queryUpdateSharePermissionsAndTarget = $query;
}
private function getSharesWithUser($shareType, $shareWiths) {
$groupedShares = [];
$query = $this->queryGetSharesWithUsers;
$query->setParameter('shareWiths', $shareWiths, IQueryBuilder::PARAM_STR_ARRAY);
$query->setParameter('shareType', $shareType);
$query->setParameter('itemTypes', ['file', 'folder'], IQueryBuilder::PARAM_STR_ARRAY);
$shares = $query->execute()->fetchAll();
// group by item_source
foreach ($shares as $share) {
if (!isset($groupedShares[$share['item_source']])) {
$groupedShares[$share['item_source']] = [];
}
$groupedShares[$share['item_source']][] = $share;
}
return $groupedShares;
}
/**
* Fix the given received share represented by the set of group shares
* and matching sub shares
*
* @param array $groupShares group share entries
* @param array $subShares sub share entries
*
* @return boolean false if the share was not repaired, true if it was
*/
private function fixThisShare($groupShares, $subShares) {
$groupSharesById = [];
foreach ($groupShares as $groupShare) {
$groupSharesById[$groupShare['id']] = $groupShare;
}
if ($this->isThisShareValid($groupSharesById, $subShares)) {
return false;
}
$targetPath = $groupShares[0]['file_target'];
// check whether the user opted out completely of all subshares
$optedOut = true;
foreach ($subShares as $subShare) {
if ((int)$subShare['permissions'] !== 0) {
$optedOut = false;
break;
}
}
$shareIds = [];
foreach ($subShares as $subShare) {
// only if the user deleted some subshares but not all, adjust the permissions of that subshare
if (!$optedOut && (int)$subShare['permissions'] === 0 && (int)$subShare['share_type'] === DefaultShareProvider::SHARE_TYPE_USERGROUP) {
// set permissions from parent group share
$permissions = $groupSharesById[$subShare['parent']]['permissions'];
// fix permissions and target directly
$query = $this->queryUpdateSharePermissionsAndTarget;
$query->setParameter('shareid', $subShare['id']);
$query->setParameter('file_target', $targetPath);
$query->setParameter('permissions', $permissions);
$query->execute();
} else {
// gather share ids for bulk target update
if ($subShare['file_target'] !== $targetPath) {
$shareIds[] = (int)$subShare['id'];
}
}
}
if (!empty($shareIds)) {
$query = $this->queryUpdateShareInBatch;
$query->setParameter('ids', $shareIds, IQueryBuilder::PARAM_INT_ARRAY);
$query->setParameter('file_target', $targetPath);
$query->execute();
}
return true;
}
/**
* Checks whether the number of group shares is balanced with the child subshares.
* If all group shares have exactly one subshare, and the target of every subshare
* is the same, then the share is valid.
* If however there is a group share entry that has no matching subshare, it means
* we're in the bogus situation and the whole share must be repaired
*
* @param array $groupSharesById
* @param array $subShares
*
* @return true if the share is valid, false if it needs repair
*/
private function isThisShareValid($groupSharesById, $subShares) {
$foundTargets = [];
// every group share needs to have exactly one matching subshare
foreach ($subShares as $subShare) {
$foundTargets[$subShare['file_target']] = true;
if (count($foundTargets) > 1) {
// not all the same target path value => invalid
return false;
}
if (isset($groupSharesById[$subShare['parent']])) {
// remove it from the list as we found it
unset($groupSharesById[$subShare['parent']]);
}
}
// if we found one subshare per group entry, the set will be empty.
// If not empty, it means that one of the group shares did not have
// a matching subshare entry.
return empty($groupSharesById);
}
/**
* Detect unmerged received shares and merge them properly
*/
private function fixUnmergedShares(IOutput $out, IUser $user) {
$groups = $this->groupManager->getUserGroupIds($user);
if (empty($groups)) {
// user is in no groups, so can't have received group shares
return;
}
$subSharesByItemSource = $this->getSharesWithUser(DefaultShareProvider::SHARE_TYPE_USERGROUP, [$user->getUID()]);
if (empty($subSharesByItemSource)) {
// nothing to repair for this user
return;
}
$groupSharesByItemSource = $this->getSharesWithUser(Constants::SHARE_TYPE_GROUP, $groups);
if (empty($groupSharesByItemSource)) {
// shouldn't happen, those invalid shares must be cleant already by RepairInvalidShares
return;
}
// because sometimes one wants to give the user more permissions than the group share
$userSharesByItemSource = $this->getSharesWithUser(Constants::SHARE_TYPE_USER, [$user->getUID()]);
foreach ($groupSharesByItemSource as $itemSource => $groupShares) {
if (!isset($subSharesByItemSource[$itemSource])) {
// no subshares for this item source, skip it
continue;
}
$subShares = $subSharesByItemSource[$itemSource];
if (isset($userSharesByItemSource[$itemSource])) {
// add it to the subshares to get a similar treatment
$subShares = array_merge($subShares, $userSharesByItemSource[$itemSource]);
}
$this->fixThisShare($groupShares, $subShares);
}
}
/**
* Count all the users
*
* @return int
*/
private function countUsers() {
$allCount = $this->userManager->countUsers();
$totalCount = 0;
foreach ($allCount as $backend => $count) {
$totalCount += $count;
}
return $totalCount;
}
public function run(IOutput $output) {
$ocVersionFromBeforeUpdate = $this->config->getSystemValue('version', '0.0.0');
if (version_compare($ocVersionFromBeforeUpdate, '9.0.4.0', '<')) {
// this situation was only possible between 9.0.0 and 9.0.3 included
$function = function(IUser $user) use ($output) {
$this->fixUnmergedShares($output, $user);
$output->advance();
};
$this->buildPreparedQueries();
$userCount = $this->countUsers();
$output->startProgress($userCount);
$this->userManager->callForAllUsers($function);
$output->finishProgress();
}
}
}

409
tests/lib/Repair/RepairUnmergedSharesTest.php Normal file
View File

@ -0,0 +1,409 @@
<?php
/**
* @author Vincent Petry <pvince81@owncloud.com>
*
* @copyright Copyright (c) 2016, ownCloud, Inc.
* @license AGPL-3.0
*
* This code is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License, version 3,
* as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License, version 3,
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/
namespace Test\Repair;
use OC\Repair\RepairUnmergedShares;
use OC\Share\Constants;
use OCP\Migration\IOutput;
use OCP\Migration\IRepairStep;
use Test\TestCase;
use OC\Share20\DefaultShareProvider;
/**
* Tests for repairing invalid shares
*
* @group DB
*
* @see \OC\Repair\RepairUnmergedShares
*/
class RepairUnmergedSharesTest extends TestCase {
/** @var IRepairStep */
private $repair;
/** @var \OCP\IDBConnection */
private $connection;
protected function setUp() {
parent::setUp();
$config = $this->getMockBuilder('OCP\IConfig')
->disableOriginalConstructor()
->getMock();
$config->expects($this->any())
->method('getSystemValue')
->with('version')
->will($this->returnValue('9.0.3.0'));
$this->connection = \OC::$server->getDatabaseConnection();
$this->deleteAllShares();
$user1 = $this->getMock('\OCP\IUser');
$user1->expects($this->any())
->method('getUID')
->will($this->returnValue('user1'));
$user2 = $this->getMock('\OCP\IUser');
$user2->expects($this->any())
->method('getUID')
->will($this->returnValue('user2'));
$users = [$user1, $user2];
$groupManager = $this->getMock('\OCP\IGroupManager');
$groupManager->expects($this->any())
->method('getUserGroupIds')
->will($this->returnValueMap([
// owner
[$user1, ['samegroup1', 'samegroup2']],
// recipient
[$user2, ['recipientgroup1', 'recipientgroup2']],
]));
$userManager = $this->getMock('\OCP\IUserManager');
$userManager->expects($this->once())
->method('countUsers')
->will($this->returnValue([2]));
$userManager->expects($this->once())
->method('callForAllUsers')
->will($this->returnCallback(function(\Closure $closure) use ($users) {
foreach ($users as $user) {
$closure($user);
}
}));
/** @var \OCP\IConfig $config */
$this->repair = new RepairUnmergedShares($config, $this->connection, $userManager, $groupManager);
}
protected function tearDown() {
$this->deleteAllShares();
parent::tearDown();
}
protected function deleteAllShares() {
$qb = $this->connection->getQueryBuilder();
$qb->delete('share')->execute();
}
private function createShare($type, $sourceId, $recipient, $targetName, $permissions, $parentId = null) {
$qb = $this->connection->getQueryBuilder();
$values = [
'share_type' => $qb->expr()->literal($type),
'share_with' => $qb->expr()->literal($recipient),
'uid_owner' => $qb->expr()->literal('user1'),
'item_type' => $qb->expr()->literal('folder'),
'item_source' => $qb->expr()->literal($sourceId),
'item_target' => $qb->expr()->literal('/' . $sourceId),
'file_source' => $qb->expr()->literal($sourceId),
'file_target' => $qb->expr()->literal($targetName),
'permissions' => $qb->expr()->literal($permissions),
'stime' => $qb->expr()->literal(time()),
];
if ($parentId !== null) {
$values['parent'] = $qb->expr()->literal($parentId);
}
$qb->insert('share')
->values($values)
->execute();
return $this->connection->lastInsertId('*PREFIX*share');
}
private function getShareById($id) {
$query = $this->connection->getQueryBuilder();
$results = $query
->select('*')
->from('share')
->where($query->expr()->eq('id', $query->expr()->literal($id)))
->execute()
->fetchAll();
if (!empty($results)) {
return $results[0];
}
return null;
}
public function sharesDataProvider() {
/**
* For all these test cases we have the following situation:
*
* - "user1" is the share owner
* - "user2" is the recipient, and member of "recipientgroup1" and "recipientgroup2"
* - "user1" is member of "samegroup1", "samegroup2" for same group tests
*/
return [
[
// #0 legitimate share:
// - outsider shares with group1, group2
// - recipient renamed, resulting in subshares
// - one subshare for each group share
// - targets of subshare all match
[
[Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup1', '/test', 31],
[Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup2', '/test', 31],
// child of the previous ones
[DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test renamed', 31, 0],
// child of the previous ones
[DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test renamed', 31, 1],
// different unrelated share
[Constants::SHARE_TYPE_GROUP, 456, 'recipientgroup1', '/test (4)', 31],
],
[
['/test', 31],
['/test', 31],
// leave them alone
['/test renamed', 31],
['/test renamed', 31],
// leave unrelated alone
['/test (4)', 31],
]
],
[
// #1 broken share:
// - outsider shares with group1, group2
// - only one subshare for two group shares
[
[Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup1', '/test', 31],
[Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup2', '/test', 31],
// child of the previous one
[DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test (2)', 31, 1],
// different unrelated share
[Constants::SHARE_TYPE_GROUP, 456, 'recipientgroup1', '/test (4)', 31],
],
[
['/test', 31],
['/test', 31],
['/test', 31],
// leave unrelated alone
['/test (4)', 31],
]
],
[
// #2 bogus share
// - outsider shares with group1, group2
// - one subshare for each group share
// - but the targets do not match when grouped
[
[Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup1', '/test', 31],
[Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup2', '/test', 31],
// child of the previous ones
[DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test (2)', 31, 0],
[DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test (3)', 31, 1],
// different unrelated share
[Constants::SHARE_TYPE_GROUP, 456, 'recipientgroup1', '/test (4)', 31],
],
[
['/test', 31],
['/test', 31],
// reset to original name
['/test', 31],
['/test', 31],
// leave unrelated alone
['/test (4)', 31],
]
],
[
// #3 bogus share
// - outsider shares with group1, group2
// - one subshare for each group share
// - first subshare not renamed (as in real world scenario)
// - but the targets do not match when grouped
[
[Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup1', '/test', 31],
[Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup2', '/test', 31],
// child of the previous ones
[DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test', 31, 0],
[DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test (2)', 31, 1],
// different unrelated share
[Constants::SHARE_TYPE_GROUP, 456, 'recipientgroup1', '/test (4)', 31],
],
[
['/test', 31],
['/test', 31],
// reset to original name
['/test', 31],
['/test', 31],
// leave unrelated alone
['/test (4)', 31],
]
],
[
// #4 bogus share:
// - outsider shares with group1, group2
// - one subshare for each group share
// - non-matching targets
// - recipient deletes one duplicate (unshare from self, permissions 0)
[
[Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup1', '/test', 31],
[Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup2', '/test', 15],
// child of the previous ones
[DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test (2)', 0, 0],
[DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test (3)', 15, 1],
// different unrelated share
[Constants::SHARE_TYPE_GROUP, 456, 'recipientgroup1', '/test (4)', 31],
],
[
['/test', 31],
['/test', 15],
// subshares repaired and permissions restored to the max allowed
['/test', 31],
['/test', 15],
// leave unrelated alone
['/test (4)', 31],
]
],
[
// #5 bogus share:
// - outsider shares with group1, group2
// - one subshare for each group share
// - non-matching targets
// - recipient deletes ALL duplicates (unshare from self, permissions 0)
[
[Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup1', '/test', 31],
[Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup2', '/test', 15],
// child of the previous ones
[DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test (2)', 0, 0],
[DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test (3)', 0, 1],
// different unrelated share
[Constants::SHARE_TYPE_GROUP, 456, 'recipientgroup1', '/test (4)', 31],
],
[
['/test', 31],
['/test', 15],
// subshares target repaired but left "deleted" as it was the user's choice
['/test', 0],
['/test', 0],
// leave unrelated alone
['/test (4)', 31],
]
],
[
// #6 bogus share:
// - outsider shares with group1, group2 and also user2
// - one subshare for each group share
// - one extra share entry for direct share to user2
// - non-matching targets
// - user share has more permissions
[
[Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup1', '/test', 1],
[Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup2', '/test', 15],
// child of the previous ones
[DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test (2)', 1, 0],
[DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test (3)', 15, 1],
[Constants::SHARE_TYPE_USER, 123, 'user2', '/test (4)', 31],
// different unrelated share
[Constants::SHARE_TYPE_GROUP, 456, 'recipientgroup1', '/test (5)', 31],
],
[
['/test', 1],
['/test', 15],
// subshares repaired
['/test', 1],
['/test', 15],
['/test', 31],
// leave unrelated alone
['/test (5)', 31],
]
],
[
// #7 legitimate share with own group:
// - insider shares with both groups the user is already in
// - no subshares in this case
[
[Constants::SHARE_TYPE_GROUP, 123, 'samegroup1', '/test', 31],
[Constants::SHARE_TYPE_GROUP, 123, 'samegroup2', '/test', 31],
// different unrelated share
[Constants::SHARE_TYPE_GROUP, 456, 'recipientgroup1', '/test (4)', 31],
],
[
// leave all alone
['/test', 31],
['/test', 31],
// leave unrelated alone
['/test (4)', 31],
]
],
[
// #7 legitimate shares:
// - group share with same group
// - group share with other group
// - user share where recipient renamed
// - user share where recipient did not rename
[
[Constants::SHARE_TYPE_GROUP, 123, 'samegroup1', '/test', 31],
[Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup1', '/test', 31],
[Constants::SHARE_TYPE_USER, 123, 'user3', '/test legit rename', 31],
[Constants::SHARE_TYPE_USER, 123, 'user4', '/test', 31],
// different unrelated share
[Constants::SHARE_TYPE_GROUP, 456, 'recipientgroup1', '/test (4)', 31],
],
[
// leave all alone
['/test', 31],
['/test', 31],
['/test legit rename', 31],
['/test', 31],
// leave unrelated alone
['/test (4)', 31],
]
],
];
}
/**
* Test merge shares from group shares
*
* @dataProvider sharesDataProvider
*/
public function testMergeGroupShares($shares, $expectedShares) {
$shareIds = [];
foreach ($shares as $share) {
// if parent
if (isset($share[5])) {
// adjust to real id
$share[5] = $shareIds[$share[5]];
} else {
$share[5] = null;
}
$shareIds[] = $this->createShare($share[0], $share[1], $share[2], $share[3], $share[4], $share[5]);
}
/** @var IOutput | \PHPUnit_Framework_MockObject_MockObject $outputMock */
$outputMock = $this->getMockBuilder('\OCP\Migration\IOutput')
->disableOriginalConstructor()
->getMock();
$this->repair->run($outputMock);
foreach ($expectedShares as $index => $expectedShare) {
$share = $this->getShareById($shareIds[$index]);
$this->assertEquals($expectedShare[0], $share['file_target']);
$this->assertEquals($expectedShare[1], $share['permissions']);
}
}
}