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

Fix RepairUnmergedShares to not skip valid repair cases 

The repair step was a bit overeager to skip repairing so it missed the
case where a group share exists without subshares but with an
additional direct user share.
This commit is contained in:
Vincent Petry 2016-07-22 10:08:05 +02:00 committed by Roeland Jago Douma
parent 3a86dd1d1a
commit 21907c4f3e
No known key found for this signature in database
GPG Key ID: 1E152838F164D13B
2 changed files with 61 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
lib/private/Repair/RepairUnmergedShares.php
View File

@ -158,6 +158,10 @@ class RepairUnmergedShares implements IRepairStep {
* @return boolean false if the share was not repaired, true if it was
*/
private function fixThisShare($groupShares, $subShares) {
if (empty($subShares)) {
return false;
}
$groupSharesById = [];
foreach ($groupShares as $groupShare) {
$groupSharesById[$groupShare['id']] = $groupShare;
@ -253,27 +257,28 @@ class RepairUnmergedShares implements IRepairStep {
return;
}
// get all subshares grouped by item source
$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()]);
if (empty($subSharesByItemSource) && empty($userSharesByItemSource)) {
// nothing to repair for this user, no need to do extra queries
return;
}
$groupSharesByItemSource = $this->getSharesWithUser(Constants::SHARE_TYPE_GROUP, $groups);
if (empty($groupSharesByItemSource) && empty($userSharesByItemSource)) {
// nothing to repair for this user
return;
}
foreach ($groupSharesByItemSource as $itemSource => $groupShares) {
if (!isset($subSharesByItemSource[$itemSource])) {
// no subshares for this item source, skip it
continue;
$subShares = [];
if (isset($subSharesByItemSource[$itemSource])) {
$subShares = $subSharesByItemSource[$itemSource];
}
$subShares = $subSharesByItemSource[$itemSource];
if (isset($userSharesByItemSource[$itemSource])) {
// add it to the subshares to get a similar treatment

44
tests/lib/Repair/RepairUnmergedSharesTest.php
View File

@ -329,7 +329,28 @@ class RepairUnmergedSharesTest extends TestCase {
]
],
[
// #7 legitimate share with own group:
// #7 bogus share:
// - outsider shares with group1 and also user2
// - no subshare at all
// - 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_USER, 123, 'user2', '/test (2)', 31],
// different unrelated share
[Constants::SHARE_TYPE_GROUP, 456, 'recipientgroup1', '/test (5)', 31],
],
[
['/test', 1],
// user share repaired
['/test', 31],
// leave unrelated alone
['/test (5)', 31],
]
],
[
// #8 legitimate share with own group:
// - insider shares with both groups the user is already in
// - no subshares in this case
[
@ -347,7 +368,7 @@ class RepairUnmergedSharesTest extends TestCase {
]
],
[
// #7 legitimate shares:
// #9 legitimate shares:
// - group share with same group
// - group share with other group
// - user share where recipient renamed
@ -370,6 +391,25 @@ class RepairUnmergedSharesTest extends TestCase {
['/test (4)', 31],
]
],
[
// #10 legitimate share:
// - outsider shares with group and user directly with different permissions
// - no subshares
// - same targets
[
[Constants::SHARE_TYPE_GROUP, 123, 'samegroup1', '/test', 1],
[Constants::SHARE_TYPE_USER, 123, 'user3', '/test', 31],
// different unrelated share
[Constants::SHARE_TYPE_GROUP, 456, 'recipientgroup1', '/test (4)', 31],
],
[
// leave all alone
['/test', 1],
['/test', 31],
// leave unrelated alone
['/test (4)', 31],
]
],
];
}