diff --git a/lib/private/share/helper.php b/lib/private/share/helper.php index 5b27f0e6f50..6059af0196d 100644 --- a/lib/private/share/helper.php +++ b/lib/private/share/helper.php @@ -79,13 +79,14 @@ class Helper extends \OC\Share\Constants { } /** - * Delete all reshares of an item + * Delete all reshares and group share children of an item * @param int $parent Id of item to delete * @param bool $excludeParent If true, exclude the parent from the delete (optional) * @param string $uidOwner The user that the parent was shared with (optional) * @param int $newParent new parent for the childrens + * @param bool $excludeGroupChildren exclude group children elements */ - public static function delete($parent, $excludeParent = false, $uidOwner = null, $newParent = null) { + public static function delete($parent, $excludeParent = false, $uidOwner = null, $newParent = null, $excludeGroupChildren = false) { $ids = array($parent); $deletedItems = array(); $changeParent = array(); @@ -94,15 +95,25 @@ class Helper extends \OC\Share\Constants { $parents = "'".implode("','", $parents)."'"; // Check the owner on the first search of reshares, useful for // finding and deleting the reshares by a single user of a group share + $params = array(); if (count($ids) == 1 && isset($uidOwner)) { - $query = \OC_DB::prepare('SELECT `id`, `share_with`, `item_type`, `share_type`, `item_target`, `file_target`, `parent`' - .' FROM `*PREFIX*share` WHERE `parent` IN ('.$parents.') AND `uid_owner` = ? AND `share_type` != ?'); - $result = $query->execute(array($uidOwner, self::$shareTypeGroupUserUnique)); + // FIXME: don't concat $parents, use Docrine's PARAM_INT_ARRAY approach + $queryString = 'SELECT `id`, `share_with`, `item_type`, `share_type`, ' . + '`item_target`, `file_target`, `parent` ' . + 'FROM `*PREFIX*share` ' . + 'WHERE `parent` IN ('.$parents.') AND `uid_owner` = ? '; + $params[] = $uidOwner; } else { - $query = \OC_DB::prepare('SELECT `id`, `share_with`, `item_type`, `share_type`, `item_target`, `file_target`, `parent`, `uid_owner`' - .' FROM `*PREFIX*share` WHERE `parent` IN ('.$parents.') AND `share_type` != ?'); - $result = $query->execute(array(self::$shareTypeGroupUserUnique)); + $queryString = 'SELECT `id`, `share_with`, `item_type`, `share_type`, ' . + '`item_target`, `file_target`, `parent`, `uid_owner` ' . + 'FROM `*PREFIX*share` WHERE `parent` IN ('.$parents.') '; } + if ($excludeGroupChildren) { + $queryString .= ' AND `share_type` != ?'; + $params[] = self::$shareTypeGroupUserUnique; + } + $query = \OC_DB::prepare($queryString); + $result = $query->execute($params); // Reset parents array, only go through loop again if items are found $parents = array(); while ($item = $result->fetchRow()) { diff --git a/lib/private/share/share.php b/lib/private/share/share.php index e85f9f06ed3..e5f350a24fb 100644 --- a/lib/private/share/share.php +++ b/lib/private/share/share.php @@ -971,7 +971,8 @@ class Share extends \OC\Share\Constants { if ($item['permissions'] & ~$permissions) { // If share permission is removed all reshares must be deleted if (($item['permissions'] & \OCP\Constants::PERMISSION_SHARE) && (~$permissions & \OCP\Constants::PERMISSION_SHARE)) { - Helper::delete($item['id'], true); + // delete all shares, keep parent and group children + Helper::delete($item['id'], true, null, null, true); } else { $ids = array(); $parents = array($item['id']); diff --git a/tests/lib/share/share.php b/tests/lib/share/share.php index b1261b0afbd..4b42036fc22 100644 --- a/tests/lib/share/share.php +++ b/tests/lib/share/share.php @@ -44,11 +44,13 @@ class Test_Share extends \Test\TestCase { $this->user2 = $this->getUniqueID('user2_'); $this->user3 = $this->getUniqueID('user3_'); $this->user4 = $this->getUniqueID('user4_'); + $this->user5 = $this->getUniqueID('user5_'); $this->groupAndUser = $this->getUniqueID('groupAndUser_'); OC_User::createUser($this->user1, 'pass'); OC_User::createUser($this->user2, 'pass'); OC_User::createUser($this->user3, 'pass'); OC_User::createUser($this->user4, 'pass'); + OC_User::createUser($this->user5, 'pass'); OC_User::createUser($this->groupAndUser, 'pass'); OC_User::setUserId($this->user1); OC_Group::clearBackends(); @@ -610,6 +612,51 @@ class Test_Share extends \Test\TestCase { $this->assertEquals(array(), OCP\Share::getItemsShared('test')); } + /** + * Test that unsharing from group will also delete all + * child entries + */ + public function testShareWithGroupThenUnshare() { + OC_User::setUserId($this->user5); + OCP\Share::shareItem( + 'test', + 'test.txt', + OCP\Share::SHARE_TYPE_GROUP, + $this->group1, + \OCP\Constants::PERMISSION_ALL + ); + + $targetUsers = array($this->user1, $this->user2, $this->user3); + + foreach($targetUsers as $targetUser) { + OC_User::setUserId($targetUser); + $items = OCP\Share::getItemsSharedWithUser( + 'test', + $targetUser, + Test_Share_Backend::FORMAT_TARGET + ); + $this->assertEquals(1, count($items)); + } + + OC_User::setUserId($this->user5); + OCP\Share::unshare( + 'test', + 'test.txt', + OCP\Share::SHARE_TYPE_GROUP, + $this->group1 + ); + + // verify that all were deleted + foreach($targetUsers as $targetUser) { + OC_User::setUserId($targetUser); + $items = OCP\Share::getItemsSharedWithUser( + 'test', + $targetUser, + Test_Share_Backend::FORMAT_TARGET + ); + $this->assertEquals(0, count($items)); + } + } public function testShareWithGroupAndUserBothHaveTheSameId() {