diff --git a/apps/federatedfilesharing/lib/federatedshareprovider.php b/apps/federatedfilesharing/lib/federatedshareprovider.php index 64e4b6de4f..78b0b66420 100644 --- a/apps/federatedfilesharing/lib/federatedshareprovider.php +++ b/apps/federatedfilesharing/lib/federatedshareprovider.php @@ -590,4 +590,15 @@ class FederatedShareProvider implements IShareProvider { // We don't handle groups here return; } + + /** + * This provider does not handle groups + * + * @param string $uid + * @param string $gid + */ + public function userDeletedFromGroup($uid, $gid) { + // We don't handle groups here + return; + } } diff --git a/lib/base.php b/lib/base.php index 6bc0fecf04..26be1161ba 100644 --- a/lib/base.php +++ b/lib/base.php @@ -776,7 +776,7 @@ class OC { public static function registerShareHooks() { if (\OC::$server->getSystemConfig()->getValue('installed')) { OC_Hook::connect('OC_User', 'post_deleteUser', 'OC\Share20\Hooks', 'post_deleteUser'); - OC_Hook::connect('OC_User', 'post_removeFromGroup', 'OC\Share\Hooks', 'post_removeFromGroup'); + OC_Hook::connect('OC_User', 'post_removeFromGroup', 'OC\Share20\Hooks', 'post_removeFromGroup'); OC_Hook::connect('OC_User', 'post_deleteGroup', 'OC\Share20\Hooks', 'post_deleteGroup'); } } diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php index b1f3b4dab8..f0de39fdad 100644 --- a/lib/private/Share20/DefaultShareProvider.php +++ b/lib/private/Share20/DefaultShareProvider.php @@ -910,4 +910,42 @@ class DefaultShareProvider implements IShareProvider { ->andWhere($qb->expr()->eq('share_with', $qb->createNamedParameter($gid))); $qb->execute(); } + + /** + * Delete custom group shares to this group for this user + * + * @param string $uid + * @param string $gid + */ + public function userDeletedFromGroup($uid, $gid) { + /* + * Get all group shares + */ + $qb = $this->dbConn->getQueryBuilder(); + $qb->select('id') + ->from('share') + ->where($qb->expr()->eq('share_type', $qb->createNamedParameter(\OCP\Share::SHARE_TYPE_GROUP))) + ->andWhere($qb->expr()->eq('share_with', $qb->createNamedParameter($gid))); + + $cursor = $qb->execute(); + $ids = []; + while($row = $cursor->fetch()) { + $ids[] = (int)$row['id']; + } + $cursor->closeCursor(); + + if (!empty($ids)) { + $chunks = array_chunk($ids, 100); + foreach ($chunks as $chunk) { + /* + * Delete all special shares wit this users for the found group shares + */ + $qb->delete('share') + ->where($qb->expr()->eq('share_type', $qb->createNamedParameter(self::SHARE_TYPE_USERGROUP))) + ->andWhere($qb->expr()->eq('share_with', $qb->createNamedParameter($uid))) + ->andWhere($qb->expr()->in('parent', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY))); + $qb->execute(); + } + } + } } diff --git a/lib/private/Share20/Hooks.php b/lib/private/Share20/Hooks.php index b391ffce8d..f29114a1b5 100644 --- a/lib/private/Share20/Hooks.php +++ b/lib/private/Share20/Hooks.php @@ -28,4 +28,8 @@ class Hooks { public static function post_deleteGroup($arguments) { \OC::$server->getShareManager()->groupDeleted($arguments['gid']); } + + public static function post_removeFromGroup($arguments) { + \OC::$server->getShareManager()->userDeletedFromGroup($arguments['uid'], $arguments['gid']); + } } diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 6f2efe167d..1ec25750cf 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -1056,6 +1056,14 @@ class Manager implements IManager { $provider->groupDeleted($gid); } + /** + * @inheritdoc + */ + public function userDeletedFromGroup($uid, $gid) { + $provider = $this->factory->getProviderForType(\OCP\Share::SHARE_TYPE_GROUP); + $provider->userDeletedFromGroup($uid, $gid); + } + /** * Get access list to a path. This means * all the users and groups that can access a given path. diff --git a/lib/private/share/hooks.php b/lib/private/share/hooks.php deleted file mode 100644 index 5faf81c5e9..0000000000 --- a/lib/private/share/hooks.php +++ /dev/null @@ -1,46 +0,0 @@ - - * @author Morris Jobke - * @author Robin McCorkell - * @author Roeland Jago Douma - * - * @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 - * - */ - -namespace OC\Share; - -class Hooks extends \OC\Share\Constants { - /** - * Function that is called after a user is removed from a group. Shares are cleaned up. - * @param array $arguments - */ - public static function post_removeFromGroup($arguments) { - $sql = 'SELECT `id`, `share_type` FROM `*PREFIX*share`' - .' WHERE (`share_type` = ? AND `share_with` = ?) OR (`share_type` = ? AND `share_with` = ?)'; - $result = \OC_DB::executeAudited($sql, array(self::SHARE_TYPE_GROUP, $arguments['gid'], - self::$shareTypeGroupUserUnique, $arguments['uid'])); - while ($item = $result->fetchRow()) { - if ($item['share_type'] == self::SHARE_TYPE_GROUP) { - // Delete all reshares by this user of the group share - Helper::delete($item['id'], true, $arguments['uid']); - } else { - Helper::delete($item['id']); - } - } - } -} diff --git a/lib/public/Share/IManager.php b/lib/public/Share/IManager.php index c43011d317..392c047176 100644 --- a/lib/public/Share/IManager.php +++ b/lib/public/Share/IManager.php @@ -164,11 +164,21 @@ interface IManager { * The group with $gid is deleted * We need to clear up all shares to this group * - * @param $gid + * @param string $gid * @since 9.1.0 */ public function groupDeleted($gid); + /** + * The user $uid is deleted from the group $gid + * All user specific group shares have to be removed + * + * @param string $uid + * @param string $gid + * @since 9.1.0 + */ + public function userDeletedFromGroup($uid, $gid); + /** * Instantiates a new share object. This is to be passed to * createShare. diff --git a/lib/public/Share/IShareProvider.php b/lib/public/Share/IShareProvider.php index 24af36e075..ac75a6f20b 100644 --- a/lib/public/Share/IShareProvider.php +++ b/lib/public/Share/IShareProvider.php @@ -166,4 +166,15 @@ interface IShareProvider { * @since 9.1.0 */ public function groupDeleted($gid); + + /** + * A user is deleted from a group + * We have to clean up all the related user specific group shares + * Providers not handling group shares should just return + * + * @param string $uid + * @param string $gid + * @since 9.1.0 + */ + public function userDeletedFromGroup($uid, $gid); } diff --git a/tests/lib/share/share.php b/tests/lib/share/share.php index 4519c33f9d..1965d80c58 100644 --- a/tests/lib/share/share.php +++ b/tests/lib/share/share.php @@ -611,162 +611,6 @@ class Test_Share extends \Test\TestCase { ); } - public function testShareWithGroup() { - // Invalid shares - $message = 'Sharing test.txt failed, because the group foobar does not exist'; - try { - OCP\Share::shareItem('test', 'test.txt', OCP\Share::SHARE_TYPE_GROUP, 'foobar', \OCP\Constants::PERMISSION_READ); - $this->fail('Exception was expected: '.$message); - } catch (Exception $exception) { - $this->assertEquals($message, $exception->getMessage()); - } - $policy = \OC::$server->getAppConfig()->getValue('core', 'shareapi_only_share_with_group_members', 'no'); - \OC::$server->getAppConfig()->setValue('core', 'shareapi_only_share_with_group_members', 'yes'); - $message = 'Sharing test.txt failed, because '.$this->user1.' is not a member of the group '.$this->group2; - try { - OCP\Share::shareItem('test', 'test.txt', OCP\Share::SHARE_TYPE_GROUP, $this->group2, \OCP\Constants::PERMISSION_READ); - $this->fail('Exception was expected: '.$message); - } catch (Exception $exception) { - $this->assertEquals($message, $exception->getMessage()); - } - \OC::$server->getAppConfig()->setValue('core', 'shareapi_only_share_with_group_members', $policy); - - // Valid share - $this->shareUserOneTestFileWithGroupOne(); - - // check if only the group share was created and not a single db-entry for each user - $statement = \OCP\DB::prepare('select `id` from `*PREFIX*share`'); - $query = $statement->execute(); - $result = $query->fetchAll(); - $this->assertSame(1, count($result)); - - - // Attempt to share again - OC_User::setUserId($this->user1); - $message = 'Sharing test.txt failed, because this item is already shared with '.$this->group1; - try { - OCP\Share::shareItem('test', 'test.txt', OCP\Share::SHARE_TYPE_GROUP, $this->group1, \OCP\Constants::PERMISSION_READ); - $this->fail('Exception was expected: '.$message); - } catch (Exception $exception) { - $this->assertEquals($message, $exception->getMessage()); - } - - // Attempt to share back to owner of group share - OC_User::setUserId($this->user2); - $message = 'Sharing failed, because the user '.$this->user1.' is the original sharer'; - try { - OCP\Share::shareItem('test', 'test.txt', OCP\Share::SHARE_TYPE_USER, $this->user1, \OCP\Constants::PERMISSION_READ); - $this->fail('Exception was expected: '.$message); - } catch (Exception $exception) { - $this->assertEquals($message, $exception->getMessage()); - } - - // Attempt to share back to group - $message = 'Sharing test.txt failed, because this item is already shared with '.$this->group1; - try { - OCP\Share::shareItem('test', 'test.txt', OCP\Share::SHARE_TYPE_GROUP, $this->group1, \OCP\Constants::PERMISSION_READ); - $this->fail('Exception was expected: '.$message); - } catch (Exception $exception) { - $this->assertEquals($message, $exception->getMessage()); - } - - // Attempt to share back to member of group - $message ='Sharing test.txt failed, because this item is already shared with '.$this->user3; - try { - OCP\Share::shareItem('test', 'test.txt', OCP\Share::SHARE_TYPE_USER, $this->user3, \OCP\Constants::PERMISSION_READ); - $this->fail('Exception was expected: '.$message); - } catch (Exception $exception) { - $this->assertEquals($message, $exception->getMessage()); - } - - // Unshare - OC_User::setUserId($this->user1); - $this->assertTrue(OCP\Share::unshare('test', 'test.txt', OCP\Share::SHARE_TYPE_GROUP, $this->group1)); - - // Valid share with same person - user then group - $this->assertTrue(OCP\Share::shareItem('test', 'test.txt', OCP\Share::SHARE_TYPE_USER, $this->user2, \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_DELETE | \OCP\Constants::PERMISSION_SHARE)); - $this->assertTrue(OCP\Share::shareItem('test', 'test.txt', OCP\Share::SHARE_TYPE_GROUP, $this->group1, \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_UPDATE)); - OC_User::setUserId($this->user2); - $this->assertEquals(array('test.txt'), OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET)); - $this->assertEquals(array(\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE | \OCP\Constants::PERMISSION_SHARE), OCP\Share::getItemSharedWith('test', 'test.txt', Test_Share_Backend::FORMAT_PERMISSIONS)); - OC_User::setUserId($this->user3); - $this->assertEquals(array('test.txt'), OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET)); - $this->assertEquals(array(\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_UPDATE), OCP\Share::getItemSharedWith('test', 'test.txt', Test_Share_Backend::FORMAT_PERMISSIONS)); - - // Valid reshare - OC_User::setUserId($this->user2); - $this->assertTrue(OCP\Share::shareItem('test', 'test.txt', OCP\Share::SHARE_TYPE_USER, $this->user4, \OCP\Constants::PERMISSION_READ)); - OC_User::setUserId($this->user4); - $this->assertEquals(array('test.txt'), OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET)); - - // Unshare from user only - OC_User::setUserId($this->user1); - $this->assertTrue(OCP\Share::unshare('test', 'test.txt', OCP\Share::SHARE_TYPE_USER, $this->user2)); - OC_User::setUserId($this->user2); - $this->assertEquals(array(\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_UPDATE), OCP\Share::getItemSharedWith('test', 'test.txt', Test_Share_Backend::FORMAT_PERMISSIONS)); - OC_User::setUserId($this->user4); - $this->assertEquals(array('test.txt'), OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET)); - - // Valid share with same person - group then user - OC_User::setUserId($this->user1); - $this->assertTrue(OCP\Share::shareItem('test', 'test.txt', OCP\Share::SHARE_TYPE_USER, $this->user2, \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_DELETE)); - OC_User::setUserId($this->user2); - $this->assertEquals(array('test.txt'), OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET)); - $this->assertEquals(array(\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE), OCP\Share::getItemSharedWith('test', 'test.txt', Test_Share_Backend::FORMAT_PERMISSIONS)); - - // Unshare from group only - OC_User::setUserId($this->user1); - $this->assertTrue(OCP\Share::unshare('test', 'test.txt', OCP\Share::SHARE_TYPE_GROUP, $this->group1)); - OC_User::setUserId($this->user2); - $this->assertEquals(array(\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_DELETE), OCP\Share::getItemSharedWith('test', 'test.txt', Test_Share_Backend::FORMAT_PERMISSIONS)); - - // Attempt user specific target conflict - OC_User::setUserId($this->user3); - \OCP\Util::connectHook('OCP\\Share', 'post_shared', 'DummyHookListener', 'listen'); - - $this->assertTrue(OCP\Share::shareItem('test', 'share.txt', OCP\Share::SHARE_TYPE_GROUP, $this->group1, \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_SHARE)); - $this->assertEquals(OCP\Share::SHARE_TYPE_GROUP, DummyHookListener::$shareType); - OC_User::setUserId($this->user2); - $to_test = OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET); - $this->assertEquals(2, count($to_test)); - $this->assertTrue(in_array('test.txt', $to_test)); - $this->assertTrue(in_array('test1.txt', $to_test)); - - // Valid reshare - $this->assertTrue(OCP\Share::shareItem('test', 'share.txt', OCP\Share::SHARE_TYPE_USER, $this->user4, \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_SHARE)); - OC_User::setUserId($this->user4); - $this->assertEquals(array('test1.txt'), OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET)); - - // Remove user from group - OC_Group::removeFromGroup($this->user2, $this->group1); - OC_User::setUserId($this->user2); - $this->assertEquals(array('test.txt'), OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET)); - OC_User::setUserId($this->user4); - $this->assertEquals(array(), OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET)); - - // Add user to group - OC_Group::addToGroup($this->user4, $this->group1); - $this->assertEquals(array('test.txt'), OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET)); - - // Unshare from self - $this->assertTrue(OCP\Share::unshareFromSelf('test', 'test.txt')); - $this->assertEquals(array(), OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET)); - OC_User::setUserId($this->user2); - $this->assertEquals(array('test.txt'), OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET)); - - // Unshare from self via source - OC_User::setUserId($this->user1); - $this->assertTrue(OCP\Share::unshareFromSelf('test', 'share.txt', true)); - $this->assertEquals(array(), OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET)); - - // Remove group - OC_Group::deleteGroup($this->group1); - OC_User::setUserId($this->user4); - $this->assertEquals(array(), OCP\Share::getItemsSharedWith('test', Test_Share_Backend::FORMAT_TARGET)); - OC_User::setUserId($this->user3); - $this->assertEquals(array(), OCP\Share::getItemsShared('test')); - } - /** * Test that unsharing from group will also delete all * child entries diff --git a/tests/lib/share20/defaultshareprovidertest.php b/tests/lib/share20/defaultshareprovidertest.php index 6acc81ccee..44a48535b9 100644 --- a/tests/lib/share20/defaultshareprovidertest.php +++ b/tests/lib/share20/defaultshareprovidertest.php @@ -2093,4 +2093,62 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->assertCount($shouldBeDeleted ? 0 : count($ids), $data); } + + public function dataUserDeletedFromGroup() { + return [ + ['group1', 'user1', true], + ['group1', 'user2', false], + ['group2', 'user1', false], + ]; + } + + /** + * Given a group share with 'group1' + * And a user specific group share with 'user1'. + * User $user is deleted from group $gid. + * + * @dataProvider dataUserDeletedFromGroup + * + * @param string $group + * @param string $user + * @param bool $toDelete + */ + public function testUserDeletedFromGroup($group, $user, $toDelete) { + $qb = $this->dbConn->getQueryBuilder(); + $qb->insert('share') + ->setValue('share_type', $qb->createNamedParameter(\OCP\Share::SHARE_TYPE_GROUP)) + ->setValue('uid_owner', $qb->createNamedParameter('owner')) + ->setValue('uid_initiator', $qb->createNamedParameter('initiator')) + ->setValue('share_with', $qb->createNamedParameter('group1')) + ->setValue('item_type', $qb->createNamedParameter('file')) + ->setValue('item_source', $qb->createNamedParameter(42)) + ->setValue('file_source', $qb->createNamedParameter(42)); + $qb->execute(); + $id1 = $qb->getLastInsertId(); + + $qb = $this->dbConn->getQueryBuilder(); + $qb->insert('share') + ->setValue('share_type', $qb->createNamedParameter(2)) + ->setValue('uid_owner', $qb->createNamedParameter('owner')) + ->setValue('uid_initiator', $qb->createNamedParameter('initiator')) + ->setValue('share_with', $qb->createNamedParameter('user1')) + ->setValue('item_type', $qb->createNamedParameter('file')) + ->setValue('item_source', $qb->createNamedParameter(42)) + ->setValue('file_source', $qb->createNamedParameter(42)) + ->setValue('parent', $qb->createNamedParameter($id1)); + $qb->execute(); + $id2 = $qb->getLastInsertId(); + + $this->provider->userDeletedFromGroup($user, $group); + + $qb = $this->dbConn->getQueryBuilder(); + $qb->select('*') + ->from('share') + ->where($qb->expr()->eq('id', $qb->createNamedParameter($id2))); + $cursor = $qb->execute(); + $data = $cursor->fetchAll(); + $cursor->closeCursor(); + + $this->assertCount($toDelete ? 0 : 1, $data); + } }