From e20db42a0c51fd7f7d028852ef60d47dd71e4adc Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Wed, 29 Apr 2020 20:40:45 +0200 Subject: [PATCH 1/2] Tags.php and the old sharing mechanism The old sharing mechanism isn't working anymore, because it was replaced by Share 2.0. Also it was nowhere used so this removes the code paths and reduces complexity. Signed-off-by: Morris Jobke --- lib/private/Share/Share.php | 54 ------------------------------------- lib/private/TagManager.php | 4 ++- lib/private/Tags.php | 27 +++---------------- lib/public/ITagManager.php | 4 +-- tests/lib/TagsTest.php | 36 ------------------------- 5 files changed, 9 insertions(+), 116 deletions(-) diff --git a/lib/private/Share/Share.php b/lib/private/Share/Share.php index c624f24312..b37d167d89 100644 --- a/lib/private/Share/Share.php +++ b/lib/private/Share/Share.php @@ -520,60 +520,6 @@ class Share extends Constants { return false; } - /** - * Get the owners of items shared with a user. - * - * @param string $user The user the items are shared with. - * @param string $type The type of the items shared with the user. - * @param boolean $includeCollections Include collection item types (optional) - * @param boolean $includeOwner include owner in the list of users the item is shared with (optional) - * @return array - * @deprecated TESTS ONLY - this methods is only used by tests - * called like this: - * \OC\Share\Share::getSharedItemsOwners($this->user, $this->type, true) - */ - public static function getSharedItemsOwners($user, $type, $includeCollections = false, $includeOwner = false) { - // First, we find out if $type is part of a collection (and if that collection is part of - // another one and so on). - $collectionTypes = []; - if (!$includeCollections || !$collectionTypes = self::getCollectionItemTypes($type)) { - $collectionTypes[] = $type; - } - - // Of these collection types, along with our original $type, we make a - // list of the ones for which a sharing backend has been registered. - // FIXME: Ideally, we wouldn't need to nest getItemsSharedWith in this loop but just call it - // with its $includeCollections parameter set to true. Unfortunately, this fails currently. - $allMaybeSharedItems = []; - foreach ($collectionTypes as $collectionType) { - if (isset(self::$backends[$collectionType])) { - $allMaybeSharedItems[$collectionType] = self::getItemsSharedWithUser( - $collectionType, - $user, - self::FORMAT_NONE - ); - } - } - - $owners = []; - if ($includeOwner) { - $owners[] = $user; - } - - // We take a look at all shared items of the given $type (or of the collections it is part of) - // and find out their owners. Then, we gather the tags for the original $type from all owners, - // and return them as elements of a list that look like "Tag (owner)". - foreach ($allMaybeSharedItems as $collectionType => $maybeSharedItems) { - foreach ($maybeSharedItems as $sharedItem) { - if (isset($sharedItem['id'])) { //workaround for https://github.com/owncloud/core/issues/2814 - $owners[] = $sharedItem['uid_owner']; - } - } - } - - return $owners; - } - /** * Get shared items from the database * @param string $itemType diff --git a/lib/private/TagManager.php b/lib/private/TagManager.php index ecc80b271f..96786c58a1 100644 --- a/lib/private/TagManager.php +++ b/lib/private/TagManager.php @@ -76,6 +76,8 @@ class TagManager implements \OCP\ITagManager { * @param string $userId user for which to retrieve the tags, defaults to the currently * logged in user * @return \OCP\ITags + * + * since 20.0.0 $includeShared isn't used anymore */ public function load($type, $defaultTags = [], $includeShared = false, $userId = null) { if (is_null($userId)) { @@ -86,6 +88,6 @@ class TagManager implements \OCP\ITagManager { } $userId = $this->userSession->getUser()->getUId(); } - return new Tags($this->mapper, $userId, $type, $defaultTags, $includeShared); + return new Tags($this->mapper, $userId, $type, $defaultTags); } } diff --git a/lib/private/Tags.php b/lib/private/Tags.php index 92263ca4fb..3fc66c69d6 100644 --- a/lib/private/Tags.php +++ b/lib/private/Tags.php @@ -119,18 +119,14 @@ class Tags implements ITags { * @param string $user The user whose data the object will operate on. * @param string $type The type of items for which tags will be loaded. * @param array $defaultTags Tags that should be created at construction. - * @param boolean $includeShared Whether to include tags for items shared with this user by others. + * + * since 20.0.0 $includeShared isn't used anymore */ - public function __construct(TagMapper $mapper, $user, $type, $defaultTags = [], $includeShared = false) { + public function __construct(TagMapper $mapper, $user, $type, $defaultTags = []) { $this->mapper = $mapper; $this->user = $user; $this->type = $type; - $this->includeShared = $includeShared; $this->owners = [$this->user]; - if ($this->includeShared) { - $this->owners = array_merge($this->owners, \OC\Share\Share::getSharedItemsOwners($this->user, $this->type, true)); - $this->backend = \OC\Share\Share::getBackend($this->type); - } $this->tags = $this->mapper->loadTags($this->owners, $this->type); if (count($defaultTags) > 0 && count($this->tags) === 0) { @@ -303,22 +299,7 @@ class Tags implements ITags { if (!is_null($result)) { while ($row = $result->fetchRow()) { - $id = (int)$row['objid']; - - if ($this->includeShared) { - // We have to check if we are really allowed to access the - // items that are tagged with $tag. To that end, we ask the - // corresponding sharing backend if the item identified by $id - // is owned by any of $this->owners. - foreach ($this->owners as $owner) { - if ($this->backend->isValidSource($id, $owner)) { - $ids[] = $id; - break; - } - } - } else { - $ids[] = $id; - } + $ids[] = (int)$row['objid']; } } diff --git a/lib/public/ITagManager.php b/lib/public/ITagManager.php index 46e1ba0629..9003beb8a2 100644 --- a/lib/public/ITagManager.php +++ b/lib/public/ITagManager.php @@ -54,11 +54,11 @@ interface ITagManager { * @see \OCP\ITags * @param string $type The type identifier e.g. 'contact' or 'event'. * @param array $defaultTags An array of default tags to be used if none are stored. - * @param boolean $includeShared Whether to include tags for items shared with this user by others. + * @param boolean $includeShared Whether to include tags for items shared with this user by others. - always false since 20.0.0 * @param string $userId user for which to retrieve the tags, defaults to the currently * logged in user * @return \OCP\ITags - * @since 6.0.0 - parameter $includeShared and $userId were added in 8.0.0 + * @since 6.0.0 - parameter $includeShared and $userId were added in 8.0.0 - $includeShared is always false since 20.0.0 */ public function load($type, $defaultTags = [], $includeShared = false, $userId = null); } diff --git a/tests/lib/TagsTest.php b/tests/lib/TagsTest.php index 38d3d611bb..69c2833433 100644 --- a/tests/lib/TagsTest.php +++ b/tests/lib/TagsTest.php @@ -24,7 +24,6 @@ namespace Test; use OCP\IUser; use OCP\IUserSession; -use OCP\Share\IShare; /** * Class TagsTest @@ -287,39 +286,4 @@ class TagsTest extends \Test\TestCase { $this->assertTrue($tagger->removeFromFavorites(1)); $this->assertEquals([], $tagger->getFavorites()); } - - public function testShareTags() { - $testTag = 'TestTag'; - \OC\Share\Share::registerBackend('test', 'Test\Share\Backend'); - - $tagger = $this->tagMgr->load('test'); - $tagger->tagAs(1, $testTag); - - - $otherUserId = $this->getUniqueID('user2_'); - $otherUser = $this->createMock(IUser::class); - $otherUser->method('getUID') - ->willReturn($otherUserId); - - \OC::$server->getUserManager()->createUser($otherUserId, 'pass'); - \OC_User::setUserId($otherUserId); - $otherUserSession = $this->createMock(IUserSession::class); - $otherUserSession - ->expects($this->any()) - ->method('getUser') - ->willReturn($otherUser); - - $otherTagMgr = new \OC\TagManager($this->tagMapper, $otherUserSession); - $otherTagger = $otherTagMgr->load('test'); - $this->assertFalse($otherTagger->hasTag($testTag)); - - \OC_User::setUserId($this->user->getUID()); - // TODO new sharing - \OC\Share\Share::shareItem('test', 1, IShare::TYPE_USER, $otherUserId, \OCP\Constants::PERMISSION_READ); - - \OC_User::setUserId($otherUserId); - $otherTagger = $otherTagMgr->load('test', [], true); // Update tags, load shared ones. - $this->assertTrue($otherTagger->hasTag($testTag)); - $this->assertContains(1, $otherTagger->getIdsForTag($testTag)); - } } From 37856cd3779cb9e2b98eb6b6825c38b32badffc6 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Wed, 29 Apr 2020 21:15:45 +0200 Subject: [PATCH 2/2] Cascading effect - more code that now is not used anymore Signed-off-by: Morris Jobke --- lib/private/Share/Share.php | 338 ------------------------------------ 1 file changed, 338 deletions(-) diff --git a/lib/private/Share/Share.php b/lib/private/Share/Share.php index b37d167d89..aed233692e 100644 --- a/lib/private/Share/Share.php +++ b/lib/private/Share/Share.php @@ -264,72 +264,6 @@ class Share extends Constants { $parameters, -1, $includeCollections); } - /** - * Share an item with a user, group, or via 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 int $permissions CRUDS - * @param string $itemSourceName - * @param \DateTime|null $expirationDate - * @return boolean|string Returns true on success or false on failure, Returns token on success for links - * @throws \OC\HintException when the share type is remote and the shareWith is invalid - * @throws \Exception - * @since 5.0.0 - parameter $itemSourceName was added in 6.0.0, parameter $expirationDate was added in 7.0.0, parameter $passwordChanged added in 9.0.0 - * @deprecated 14.0.0 TESTS ONLY - this methods is as of 2018-06 only used by tests - * called like this: - * \OC\Share\Share::shareItem('test', 1, IShare::TYPE_USER, $otherUserId, \OCP\Constants::PERMISSION_READ); - */ - public static function shareItem($itemType, $itemSource, $shareType, $shareWith, $permissions) { - $backend = self::getBackend($itemType); - - if ($backend->isShareTypeAllowed($shareType) === false) { - $message = 'Sharing failed, because the backend does not allow shares from type %i'; - throw new \Exception(sprintf($message, $shareType)); - } - - $uidOwner = \OC_User::getUser(); - - // Verify share type and sharing conditions are met - if ($shareType === IShare::TYPE_USER) { - if ($shareWith == $uidOwner) { - $message = 'Sharing failed, because you can not share with yourself'; - throw new \Exception($message); - } - if (!\OC::$server->getUserManager()->userExists($shareWith)) { - $message = 'Sharing failed, because the user %s does not exist'; - throw new \Exception(sprintf($message, $shareWith)); - } - // Check if the item source is already shared with the user, either from the same owner or a different user - if ($checkExists = self::getItems($itemType, $itemSource, self::$shareTypeUserAndGroups, - $shareWith, null, self::FORMAT_NONE, null, 1, true, true)) { - // Only allow the same share to occur again if it is the same - // owner and is not a user share, this use case is for increasing - // permissions for a specific user - if ($checkExists['uid_owner'] != $uidOwner || $checkExists['share_type'] == $shareType) { - $message = 'Sharing failed, because this item is already shared with %s'; - throw new \Exception(sprintf($message, $shareWith)); - } - } - if ($checkExists = self::getItems($itemType, $itemSource, IShare::TYPE_USER, - $shareWith, null, self::FORMAT_NONE, null, 1, true, true)) { - // Only allow the same share to occur again if it is the same - // owner and is not a user share, this use case is for increasing - // permissions for a specific user - if ($checkExists['uid_owner'] != $uidOwner || $checkExists['share_type'] == $shareType) { - $message = 'Sharing failed, because this item is already shared with user %s'; - throw new \Exception(sprintf($message, $shareWith)); - } - } - } - - // Put the item into the database - $result = self::put('test', $itemSource, IShare::TYPE_USER, $shareWith, $uidOwner, $permissions); - - return $result ? true : false; - } - /** * Unshare an item from a user, group, or delete a private link * @param string $itemType @@ -980,278 +914,6 @@ class Share extends Constants { return $result; } - /** - * Put shared item into the database - * @param string $itemType Item type - * @param string $itemSource Item source - * @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 $uidOwner User that is the owner of shared item - * @param int $permissions CRUDS permissions - * @throws \Exception - * @return mixed id of the new share or false - * @deprecated TESTS ONLY - this methods is only used by tests - * called like this: - * self::put('test', $itemSource, IShare::TYPE_USER, $shareWith, $uidOwner, $permissions); - */ - private static function put($itemType, $itemSource, $shareType, $shareWith, $uidOwner, - $permissions) { - $queriesToExecute = []; - $suggestedItemTarget = null; - $groupFileTarget = $fileTarget = $suggestedFileTarget = $filePath = ''; - $groupItemTarget = $itemTarget = $fileSource = $parent = 0; - - $result = self::checkReshare('test', $itemSource, IShare::TYPE_USER, $shareWith, $uidOwner, $permissions, null, null); - if (!empty($result)) { - $parent = $result['parent']; - $itemSource = $result['itemSource']; - $fileSource = $result['fileSource']; - $suggestedItemTarget = $result['suggestedItemTarget']; - $suggestedFileTarget = $result['suggestedFileTarget']; - $filePath = $result['filePath']; - } - - $isGroupShare = false; - $users = [$shareWith]; - $itemTarget = Helper::generateTarget($itemType, $itemSource, $shareType, $shareWith, $uidOwner, - $suggestedItemTarget); - - $run = true; - $error = ''; - $preHookData = [ - 'itemType' => $itemType, - 'itemSource' => $itemSource, - 'shareType' => $shareType, - 'uidOwner' => $uidOwner, - 'permissions' => $permissions, - 'fileSource' => $fileSource, - 'expiration' => null, - 'token' => null, - 'run' => &$run, - 'error' => &$error - ]; - - $preHookData['itemTarget'] = $itemTarget; - $preHookData['shareWith'] = $shareWith; - - \OC_Hook::emit(\OCP\Share::class, 'pre_shared', $preHookData); - - if ($run === false) { - throw new \Exception($error); - } - - foreach ($users as $user) { - $sourceId = ($itemType === 'file' || $itemType === 'folder') ? $fileSource : $itemSource; - $sourceExists = self::getItemSharedWithBySource($itemType, $sourceId, self::FORMAT_NONE, null, true, $user); - - $userShareType = $shareType; - - if ($sourceExists && $sourceExists['item_source'] === $itemSource) { - $fileTarget = $sourceExists['file_target']; - $itemTarget = $sourceExists['item_target']; - } elseif (!$sourceExists) { - $itemTarget = Helper::generateTarget($itemType, $itemSource, $userShareType, $user, - $uidOwner, $suggestedItemTarget, $parent); - if (isset($fileSource)) { - $fileTarget = Helper::generateTarget('file', $filePath, $userShareType, - $user, $uidOwner, $suggestedFileTarget, $parent); - } else { - $fileTarget = null; - } - } else { - - // group share which doesn't exists until now, check if we need a unique target for this user - - $itemTarget = Helper::generateTarget($itemType, $itemSource, IShare::TYPE_USER, $user, - $uidOwner, $suggestedItemTarget, $parent); - - // do we also need a file target - if (isset($fileSource)) { - $fileTarget = Helper::generateTarget('file', $filePath, IShare::TYPE_USER, $user, - $uidOwner, $suggestedFileTarget, $parent); - } else { - $fileTarget = null; - } - - if (($itemTarget === $groupItemTarget) && - (!isset($fileSource) || $fileTarget === $groupFileTarget)) { - continue; - } - } - - $queriesToExecute[] = [ - 'itemType' => $itemType, - 'itemSource' => $itemSource, - 'itemTarget' => $itemTarget, - 'shareType' => $userShareType, - 'shareWith' => $user, - 'uidOwner' => $uidOwner, - 'permissions' => $permissions, - 'shareTime' => time(), - 'fileSource' => $fileSource, - 'fileTarget' => $fileTarget, - 'token' => null, - 'parent' => $parent, - 'expiration' => null, - ]; - } - - $id = false; - - foreach ($queriesToExecute as $shareQuery) { - $shareQuery['parent'] = $parent; - $id = self::insertShare($shareQuery); - } - - $postHookData = [ - 'itemType' => $itemType, - 'itemSource' => $itemSource, - 'parent' => $parent, - 'shareType' => $shareType, - 'uidOwner' => $uidOwner, - 'permissions' => $permissions, - 'fileSource' => $fileSource, - 'id' => $parent, - 'token' => null, - 'expirationDate' => null, - ]; - - $postHookData['shareWith'] = $isGroupShare ? $shareWith['group'] : $shareWith; - $postHookData['itemTarget'] = $isGroupShare ? $groupItemTarget : $itemTarget; - $postHookData['fileTarget'] = $isGroupShare ? $groupFileTarget : $fileTarget; - - \OC_Hook::emit(\OCP\Share::class, 'post_shared', $postHookData); - - - return $id ? $id : false; - } - - /** - * @param string $itemType - * @param string $itemSource - * @param int $shareType - * @param string $shareWith - * @param string $uidOwner - * @param int $permissions - * @param string|null $itemSourceName - * @param null|\DateTime $expirationDate - * @deprecated TESTS ONLY - this methods is only used by tests - * called like this: - * self::checkReshare('test', $itemSource, IShare::TYPE_USER, $shareWith, $uidOwner, $permissions, null, null); - */ - private static function checkReshare($itemType, $itemSource, $shareType, $shareWith, $uidOwner, $permissions, $itemSourceName, $expirationDate) { - $backend = self::getBackend($itemType); - - $result = []; - - $column = ($itemType === 'file' || $itemType === 'folder') ? 'file_source' : 'item_source'; - - $checkReshare = self::getItemSharedWithBySource($itemType, $itemSource, self::FORMAT_NONE, null, true); - if ($checkReshare) { - // Check if attempting to share back to owner - if ($checkReshare['uid_owner'] == $shareWith) { - $message = 'Sharing %1$s failed, because the user %2$s is the original sharer'; - throw new \Exception(sprintf($message, $itemSourceName, $shareWith)); - } - } - - if ($checkReshare && $checkReshare['uid_owner'] !== \OC_User::getUser()) { - // Check if share permissions is granted - if (self::isResharingAllowed() && (int)$checkReshare['permissions'] & \OCP\Constants::PERMISSION_SHARE) { - if (~(int)$checkReshare['permissions'] & $permissions) { - $message = 'Sharing %1$s failed, because the permissions exceed permissions granted to %2$s'; - throw new \Exception(sprintf($message, $itemSourceName, $uidOwner)); - } else { - // TODO Don't check if inside folder - $result['parent'] = $checkReshare['id']; - - $result['expirationDate'] = $expirationDate; - // $checkReshare['expiration'] could be null and then is always less than any value - if (isset($checkReshare['expiration']) && $checkReshare['expiration'] < $expirationDate) { - $result['expirationDate'] = $checkReshare['expiration']; - } - - // only suggest the same name as new target if it is a reshare of the - // same file/folder and not the reshare of a child - if ($checkReshare[$column] === $itemSource) { - $result['filePath'] = $checkReshare['file_target']; - $result['itemSource'] = $checkReshare['item_source']; - $result['fileSource'] = $checkReshare['file_source']; - $result['suggestedItemTarget'] = $checkReshare['item_target']; - $result['suggestedFileTarget'] = $checkReshare['file_target']; - } else { - $result['filePath'] = ($backend instanceof \OCP\Share_Backend_File_Dependent) ? $backend->getFilePath($itemSource, $uidOwner) : null; - $result['suggestedItemTarget'] = null; - $result['suggestedFileTarget'] = null; - $result['itemSource'] = $itemSource; - $result['fileSource'] = ($backend instanceof \OCP\Share_Backend_File_Dependent) ? $itemSource : null; - } - } - } else { - $message = 'Sharing %s failed, because resharing is not allowed'; - throw new \Exception(sprintf($message, $itemSourceName)); - } - } else { - $result['parent'] = null; - $result['suggestedItemTarget'] = null; - $result['suggestedFileTarget'] = null; - $result['itemSource'] = $itemSource; - $result['expirationDate'] = $expirationDate; - if (!$backend->isValidSource($itemSource, $uidOwner)) { - $message = 'Sharing %1$s failed, because the sharing backend for ' - .'%2$s could not find its source'; - throw new \Exception(sprintf($message, $itemSource, $itemType)); - } - if ($backend instanceof \OCP\Share_Backend_File_Dependent) { - $result['filePath'] = $backend->getFilePath($itemSource, $uidOwner); - $meta = \OC\Files\Filesystem::getFileInfo($result['filePath']); - $result['fileSource'] = $meta['fileid']; - if ($result['fileSource'] == -1) { - $message = 'Sharing %s failed, because the file could not be found in the file cache'; - throw new \Exception(sprintf($message, $itemSource)); - } - } else { - $result['filePath'] = null; - $result['fileSource'] = null; - } - } - - return $result; - } - - /** - * - * @param array $shareData - * @return mixed false in case of a failure or the id of the new share - */ - private static function insertShare(array $shareData) { - $query = \OC_DB::prepare('INSERT INTO `*PREFIX*share` (' - .' `item_type`, `item_source`, `item_target`, `share_type`,' - .' `share_with`, `uid_owner`, `permissions`, `stime`, `file_source`,' - .' `file_target`, `token`, `parent`, `expiration`) VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?)'); - $query->bindValue(1, $shareData['itemType']); - $query->bindValue(2, $shareData['itemSource']); - $query->bindValue(3, $shareData['itemTarget']); - $query->bindValue(4, $shareData['shareType']); - $query->bindValue(5, $shareData['shareWith']); - $query->bindValue(6, $shareData['uidOwner']); - $query->bindValue(7, $shareData['permissions']); - $query->bindValue(8, $shareData['shareTime']); - $query->bindValue(9, $shareData['fileSource']); - $query->bindValue(10, $shareData['fileTarget']); - $query->bindValue(11, $shareData['token']); - $query->bindValue(12, $shareData['parent']); - $query->bindValue(13, $shareData['expiration'], 'datetime'); - $result = $query->execute(); - - $id = false; - if ($result) { - $id = \OC::$server->getDatabaseConnection()->lastInsertId('*PREFIX*share'); - } - - return $id; - } - /** * construct select statement * @param int $format