From 83a75c670bb52efbee498418398f03ab12301840 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Sun, 29 Nov 2020 22:50:30 +0100 Subject: [PATCH] Replace static call to Share::unshare with ShareManager->deleteShare in tests And then cleanup all the code that is dead then... Signed-off-by: Morris Jobke --- .../files_sharing/tests/SharedStorageTest.php | 4 +- lib/private/Share/Helper.php | 104 ------------- lib/private/Share/Share.php | 138 ------------------ tests/lib/Files/ViewTest.php | 4 +- tests/lib/Share/HelperTest.php | 73 --------- 5 files changed, 4 insertions(+), 319 deletions(-) diff --git a/apps/files_sharing/tests/SharedStorageTest.php b/apps/files_sharing/tests/SharedStorageTest.php index 74975e7cf3..bd8b18f2b8 100644 --- a/apps/files_sharing/tests/SharedStorageTest.php +++ b/apps/files_sharing/tests/SharedStorageTest.php @@ -302,9 +302,7 @@ class SharedStorageTest extends TestCase { //cleanup self::loginHelper(self::TEST_FILES_SHARING_API_USER1); - $result = \OC\Share\Share::unshare('folder', $fileinfoFolder['fileid'], IShare::TYPE_USER, - self::TEST_FILES_SHARING_API_USER2); - $this->assertTrue($result); + $this->shareManager->deleteShare($share); } public function testFopenWithUpdateOnlyPermission() { diff --git a/lib/private/Share/Helper.php b/lib/private/Share/Helper.php index 06f9baa4f5..191e410d99 100644 --- a/lib/private/Share/Helper.php +++ b/lib/private/Share/Helper.php @@ -30,94 +30,8 @@ namespace OC\Share; -use OC\HintException; -use OCP\DB\QueryBuilder\IQueryBuilder; - class Helper extends \OC\Share\Constants { - /** - * 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, $excludeGroupChildren = false) { - $ids = [$parent]; - $deletedItems = []; - $changeParent = []; - $parents = [$parent]; - while (!empty($parents)) { - $query = \OC::$server->getDatabaseConnection()->getQueryBuilder(); - $query->select( - 'id', 'share_with', 'item_type', 'share_type', - 'item_target', 'file_target', 'parent' - ) - ->from('share') - ->where($query->expr()->in('parent', $query->createNamedParameter( - $parents, IQueryBuilder::PARAM_INT_ARRAY - ))); - - if (count($ids) === 1 && isset($uidOwner)) { - // Check the owner on the first search of reshares, useful for - // finding and deleting the reshares by a single user of a group share - $query->andWhere($query->expr()->eq('uid_owner', $uidOwner)); - } - - if ($excludeGroupChildren) { - $query->andWhere($query->expr()->eq('share_type', self::$shareTypeGroupUserUnique)); - } - - $result = $query->execute(); - // Reset parents array, only go through loop again if items are found - $parents = []; - while ($item = $result->fetch()) { - $tmpItem = [ - 'id' => $item['id'], - 'shareWith' => $item['share_with'], - 'itemTarget' => $item['item_target'], - 'itemType' => $item['item_type'], - 'shareType' => (int)$item['share_type'], - ]; - if (isset($item['file_target'])) { - $tmpItem['fileTarget'] = $item['file_target']; - } - // if we have a new parent for the child we remember the child - // to update the parent, if not we add it to the list of items - // which should be deleted - if ($newParent !== null) { - $changeParent[] = $item['id']; - } else { - $deletedItems[] = $tmpItem; - $ids[] = $item['id']; - $parents[] = $item['id']; - } - } - $result->closeCursor(); - } - if ($excludeParent) { - unset($ids[0]); - } - - if (!empty($changeParent)) { - $query = \OC::$server->getDatabaseConnection()->getQueryBuilder(); - $query->update('share') - ->set('parent', $query->createNamedParameter($newParent, IQueryBuilder::PARAM_INT)) - ->where($query->expr()->in('id', $query->createNamedParameter($changeParent, IQueryBuilder::PARAM_INT_ARRAY))); - $query->execute(); - } - - if (!empty($ids)) { - $query = \OC::$server->getDatabaseConnection()->getQueryBuilder(); - $query->delete('share') - ->where($query->expr()->in('id', $query->createNamedParameter($ids, IQueryBuilder::PARAM_INT_ARRAY))); - $query->execute(); - } - - return $deletedItems; - } - /** * get default expire settings defined by the admin * @return array contains 'defaultExpireDateSet', 'enforceExpireDate', 'expireAfterDays' @@ -203,24 +117,6 @@ class Helper extends \OC\Share\Constants { return $remote; } - /** - * split user and remote from federated cloud id - * - * @param string $id - * @return string[] - * @throws HintException - */ - public static function splitUserRemote($id) { - try { - $cloudId = \OC::$server->getCloudIdManager()->resolveCloudId($id); - return [$cloudId->getUser(), $cloudId->getRemote()]; - } catch (\InvalidArgumentException $e) { - $l = \OC::$server->getL10N('core'); - $hint = $l->t('Invalid Federated Cloud ID'); - throw new HintException('Invalid Federated Cloud ID', $hint, 0, $e); - } - } - /** * check if two federated cloud IDs refer to the same user * diff --git a/lib/private/Share/Share.php b/lib/private/Share/Share.php index eb1ba2135b..fdefb3bf25 100644 --- a/lib/private/Share/Share.php +++ b/lib/private/Share/Share.php @@ -251,85 +251,6 @@ class Share extends Constants { null, -1, $includeCollections); } - /** - * Unshare an item from a user, group, or delete a private link - * @param string $itemType - * @param string $itemSource - * @param int $shareType SHARE_TYPE_USER, SHARE_TYPE_GROUP, or SHARE_TYPE_LINK - * @param string $shareWith User or group the item is being shared with - * @param string $owner owner of the share, if null the current user is used - * @return boolean true on success or false on failure - */ - public static function unshare($itemType, $itemSource, $shareType, $shareWith, $owner = null) { - - // check if it is a valid itemType - self::getBackend($itemType); - - $items = self::getItemSharedWithUser($itemType, $itemSource, $shareWith, $owner, $shareType); - - $toDelete = []; - $newParent = null; - $currentUser = $owner ? $owner : \OC_User::getUser(); - foreach ($items as $item) { - // delete the item with the expected share_type and owner - if ((int)$item['share_type'] === (int)$shareType && $item['uid_owner'] === $currentUser) { - $toDelete = $item; - // if there is more then one result we don't have to delete the children - // but update their parent. For group shares the new parent should always be - // the original group share and not the db entry with the unique name - } elseif ((int)$item['share_type'] === self::$shareTypeGroupUserUnique) { - $newParent = $item['parent']; - } else { - $newParent = $item['id']; - } - } - - if (!empty($toDelete)) { - self::unshareItem($toDelete, $newParent); - return true; - } - return false; - } - - /** - * Unshares a share given a share data array - * @param array $item Share data (usually database row) - * @param int $newParent parent ID - * @return null - */ - protected static function unshareItem(array $item, $newParent = null) { - $shareType = (int)$item['share_type']; - $shareWith = null; - if ($shareType !== IShare::TYPE_LINK) { - $shareWith = $item['share_with']; - } - - // Pass all the vars we have for now, they may be useful - $hookParams = [ - 'id' => $item['id'], - 'itemType' => $item['item_type'], - 'itemSource' => $item['item_source'], - 'shareType' => $shareType, - 'shareWith' => $shareWith, - 'itemParent' => $item['parent'], - 'uidOwner' => $item['uid_owner'], - ]; - if ($item['item_type'] === 'file' || $item['item_type'] === 'folder') { - $hookParams['fileSource'] = $item['file_source']; - $hookParams['fileTarget'] = $item['file_target']; - } - - \OC_Hook::emit(\OCP\Share::class, 'pre_unshare', $hookParams); - $deletedShares = Helper::delete($item['id'], false, null, $newParent); - $deletedShares[] = $hookParams; - $hookParams['deletedShares'] = $deletedShares; - \OC_Hook::emit(\OCP\Share::class, 'post_unshare', $hookParams); - if ((int)$item['share_type'] === IShare::TYPE_REMOTE && \OC::$server->getUserSession()->getUser()) { - list(, $remote) = Helper::splitUserRemote($item['share_with']); - self::sendRemoteUnshare($remote, $item['id'], $item['token']); - } - } - /** * Get the backend class for the specified item type * @param string $itemType @@ -964,65 +885,6 @@ class Share extends Constants { return $url; } - /** - * try http post first with https and then with http as a fallback - * - * @param string $remoteDomain - * @param string $urlSuffix - * @param array $fields post parameters - * @return array - */ - private static function tryHttpPostToShareEndpoint($remoteDomain, $urlSuffix, array $fields) { - $protocol = 'https://'; - $result = [ - 'success' => false, - 'result' => '', - ]; - $try = 0; - $discoveryService = \OC::$server->query(\OCP\OCS\IDiscoveryService::class); - while ($result['success'] === false && $try < 2) { - $federationEndpoints = $discoveryService->discover($protocol . $remoteDomain, 'FEDERATED_SHARING'); - $endpoint = isset($federationEndpoints['share']) ? $federationEndpoints['share'] : '/ocs/v2.php/cloud/shares'; - $client = \OC::$server->getHTTPClientService()->newClient(); - - try { - $response = $client->post( - $protocol . $remoteDomain . $endpoint . $urlSuffix . '?format=' . self::RESPONSE_FORMAT, - [ - 'body' => $fields, - 'connect_timeout' => 10, - ] - ); - - $result = ['success' => true, 'result' => $response->getBody()]; - } catch (\Exception $e) { - $result = ['success' => false, 'result' => $e->getMessage()]; - } - - $try++; - $protocol = 'http://'; - } - - return $result; - } - - /** - * send server-to-server unshare to remote server - * - * @param string $remote url - * @param int $id share id - * @param string $token - * @return bool - */ - private static function sendRemoteUnshare($remote, $id, $token) { - $url = rtrim($remote, '/'); - $fields = ['token' => $token, 'format' => 'json']; - $url = self::removeProtocolFromUrl($url); - $result = self::tryHttpPostToShareEndpoint($url, '/'.$id.'/unshare', $fields); - $status = json_decode($result['result'], true); - - return ($result['success'] && ($status['ocs']['meta']['statuscode'] === 100 || $status['ocs']['meta']['statuscode'] === 200)); - } /** * @return int diff --git a/tests/lib/Files/ViewTest.php b/tests/lib/Files/ViewTest.php index 139516ba37..f18af484f8 100644 --- a/tests/lib/Files/ViewTest.php +++ b/tests/lib/Files/ViewTest.php @@ -1674,6 +1674,8 @@ class ViewTest extends \Test\TestCase { ->setSharedBy($this->user) ->setShareType(IShare::TYPE_USER) ->setPermissions(\OCP\Constants::PERMISSION_READ) + ->setId(42) + ->setProviderId('foo') ->setNode($shareDir); $shareManager->createShare($share); @@ -1681,7 +1683,7 @@ class ViewTest extends \Test\TestCase { $this->assertFalse($view->rename('mount1', 'shareddir/sub'), 'Cannot move mount point into shared folder'); $this->assertFalse($view->rename('mount1', 'shareddir/sub/sub2'), 'Cannot move mount point into shared subfolder'); - $this->assertTrue(\OC\Share\Share::unshare('folder', $fileId, IShare::TYPE_USER, 'test2')); + $shareManager->deleteShare($share); $userObject->delete(); } diff --git a/tests/lib/Share/HelperTest.php b/tests/lib/Share/HelperTest.php index 44bd70e7e2..8d01040027 100644 --- a/tests/lib/Share/HelperTest.php +++ b/tests/lib/Share/HelperTest.php @@ -55,79 +55,6 @@ class HelperTest extends \Test\TestCase { $this->assertSame($expected, $result); } - public function dataTestSplitUserRemote() { - $userPrefix = ['user@name', 'username']; - $protocols = ['', 'http://', 'https://']; - $remotes = [ - 'localhost', - 'local.host', - 'dev.local.host', - 'dev.local.host/path', - 'dev.local.host/at@inpath', - '127.0.0.1', - '::1', - '::192.0.2.128', - '::192.0.2.128/at@inpath', - ]; - - $testCases = []; - foreach ($userPrefix as $user) { - foreach ($remotes as $remote) { - foreach ($protocols as $protocol) { - $baseUrl = $user . '@' . $protocol . $remote; - - $testCases[] = [$baseUrl, $user, $protocol . $remote]; - $testCases[] = [$baseUrl . '/', $user, $protocol . $remote]; - $testCases[] = [$baseUrl . '/index.php', $user, $protocol . $remote]; - $testCases[] = [$baseUrl . '/index.php/s/token', $user, $protocol . $remote]; - } - } - } - return $testCases; - } - - /** - * @dataProvider dataTestSplitUserRemote - * - * @param string $remote - * @param string $expectedUser - * @param string $expectedUrl - */ - public function testSplitUserRemote($remote, $expectedUser, $expectedUrl) { - list($remoteUser, $remoteUrl) = \OC\Share\Helper::splitUserRemote($remote); - $this->assertSame($expectedUser, $remoteUser); - $this->assertSame($expectedUrl, $remoteUrl); - } - - public function dataTestSplitUserRemoteError() { - return [ - // Invalid path - ['user@'], - - // Invalid user - ['@server'], - ['us/er@server'], - ['us:er@server'], - - // Invalid splitting - ['user'], - [''], - ['us/erserver'], - ['us:erserver'], - ]; - } - - /** - * @dataProvider dataTestSplitUserRemoteError - * - * @param string $id - */ - public function testSplitUserRemoteError($id) { - $this->expectException(\OC\HintException::class); - - \OC\Share\Helper::splitUserRemote($id); - } - /** * @dataProvider dataTestCompareServerAddresses *