summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMorris Jobke <hey@morrisjobke.de>2020-11-30 11:11:59 +0100
committerGitHub <noreply@github.com>2020-11-30 11:11:59 +0100
commit356350ffe82d76a6e101b6e85931ec929c22d0df (patch)
treee04ecf9ad8f3ab24b6b6af0371507f6c3100e2c3
parent9be5caa9371bc2acad3d0b517d0aa069c6635c77 (diff)
parent83a75c670bb52efbee498418398f03ab12301840 (diff)
downloadnextcloud-server-356350ffe82d76a6e101b6e85931ec929c22d0df.tar.gz
nextcloud-server-356350ffe82d76a6e101b6e85931ec929c22d0df.zip
Merge pull request #24440 from nextcloud/techdebt/noid/replace-static-unshare-call-in-test-with-sharemanager
Replace static call to Share::unshare with ShareManager->deleteShare in tests
-rw-r--r--apps/files_sharing/tests/SharedStorageTest.php4
-rw-r--r--lib/private/Share/Helper.php104
-rw-r--r--lib/private/Share/Share.php138
-rw-r--r--tests/lib/Files/ViewTest.php4
-rw-r--r--tests/lib/Share/HelperTest.php73
5 files changed, 4 insertions, 319 deletions
diff --git a/apps/files_sharing/tests/SharedStorageTest.php b/apps/files_sharing/tests/SharedStorageTest.php
index 74975e7cf31..bd8b18f2b89 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 06f9baa4f57..191e410d990 100644
--- a/lib/private/Share/Helper.php
+++ b/lib/private/Share/Helper.php
@@ -30,95 +30,9 @@
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'
*/
@@ -204,24 +118,6 @@ class Helper extends \OC\Share\Constants {
}
/**
- * 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
*
* @param string $user1
diff --git a/lib/private/Share/Share.php b/lib/private/Share/Share.php
index eb1ba2135b8..fdefb3bf259 100644
--- a/lib/private/Share/Share.php
+++ b/lib/private/Share/Share.php
@@ -252,85 +252,6 @@ class Share extends Constants {
}
/**
- * 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
* @throws \Exception
@@ -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 139516ba374..f18af484f87 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 44bd70e7e29..8d010400273 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
*