diff options
| -rw-r--r-- | lib/private/Repair/RepairUnmergedShares.php | 27 | ||||
| -rw-r--r-- | tests/lib/Repair/RepairUnmergedSharesTest.php | 44 |
2 files changed, 58 insertions, 13 deletions
diff --git a/lib/private/Repair/RepairUnmergedShares.php b/lib/private/Repair/RepairUnmergedShares.php index 15a4762da79..353877bb873 100644 --- a/lib/private/Repair/RepairUnmergedShares.php +++ b/lib/private/Repair/RepairUnmergedShares.php @@ -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 + + // 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)) { - // shouldn't happen, those invalid shares must be cleant already by RepairInvalidShares + if (empty($groupSharesByItemSource) && empty($userSharesByItemSource)) { + // nothing to repair for this user 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 = []; + if (isset($subSharesByItemSource[$itemSource])) { + $subShares = $subSharesByItemSource[$itemSource]; } - $subShares = $subSharesByItemSource[$itemSource]; if (isset($userSharesByItemSource[$itemSource])) { // add it to the subshares to get a similar treatment diff --git a/tests/lib/Repair/RepairUnmergedSharesTest.php b/tests/lib/Repair/RepairUnmergedSharesTest.php index 20dabac2e6b..fe9b3e5b96f 100644 --- a/tests/lib/Repair/RepairUnmergedSharesTest.php +++ b/tests/lib/Repair/RepairUnmergedSharesTest.php @@ -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], + ] + ], ]; } |