Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable10] Update public link share in transfer ownership command #31176

Merged
merged 1 commit into from
May 30, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 1 addition & 29 deletions apps/files/lib/Command/TransferOwnership.php
Original file line number Diff line number Diff line change
Expand Up @@ -299,35 +299,7 @@ private function restoreShares(OutputInterface $output) {

foreach($this->shares as $share) {
try {
if ($share->getSharedWith() === $this->destinationUser) {
// Unmount the shares before deleting, so we don't try to get the storage later on.
$shareMountPoint = $this->mountManager->find('/' . $this->destinationUser . '/files' . $share->getTarget());
if ($shareMountPoint) {
$this->mountManager->removeMount($shareMountPoint->getMountPoint());
}
$this->shareManager->deleteShare($share);
} else {
if ($share->getShareOwner() === $this->sourceUser) {
$share->setShareOwner($this->destinationUser);
}
if ($share->getSharedBy() === $this->sourceUser) {
$share->setSharedBy($this->destinationUser);
}
/*
* If the share is already moved then updateShare would cause exception
* This can happen if the folder is shared and file(s) inside the folder
* has shares, for example public link
*/
if ($share->getShareType() === \OCP\Share::SHARE_TYPE_LINK) {
$sharePath = ltrim($share->getNode()->getPath(), '/');
if (strpos($sharePath, $this->finalTarget) !== false) {
//The share is already moved
continue;
}
}

$this->shareManager->updateShare($share);
}
$this->shareManager->transferShare($share, $this->sourceUser, $this->destinationUser, $this->finalTarget);
} catch (\OCP\Files\NotFoundException $e) {
$output->writeln('<error>Share with id ' . $share->getId() . ' points at deleted file, skipping</error>');
} catch (\Exception $e) {
Expand Down
3 changes: 2 additions & 1 deletion lib/private/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -869,7 +869,8 @@ public function __construct($webRoot, \OC\Config $config) {
$factory,
$c->getUserManager(),
$c->getLazyRootFolder(),
$c->getEventDispatcher()
$c->getEventDispatcher(),
new View('/')
);

return $manager;
Expand Down
96 changes: 95 additions & 1 deletion lib/private/Share20/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

use OC\Cache\CappedMemoryCache;
use OC\Files\Mount\MoveableMount;
use OC\Files\View;
use OCP\Files\File;
use OCP\Files\Folder;
use OCP\Files\IRootFolder;
Expand All @@ -42,8 +43,10 @@
use OCP\Security\ISecureRandom;
use OCP\Share\Exceptions\GenericShareException;
use OCP\Share\Exceptions\ShareNotFound;
use OCP\Share\Exceptions\TransferSharesException;
use OCP\Share\IManager;
use OCP\Share\IProviderFactory;
use OCP\Share\IShare;
use Symfony\Component\EventDispatcher\EventDispatcher;
use Symfony\Component\EventDispatcher\GenericEvent;

Expand Down Expand Up @@ -76,6 +79,8 @@ class Manager implements IManager {
private $sharingDisabledForUsersCache;
/** @var EventDispatcher */
private $eventDispatcher;
/** @var View */
private $view;


/**
Expand Down Expand Up @@ -103,7 +108,8 @@ public function __construct(
IProviderFactory $factory,
IUserManager $userManager,
IRootFolder $rootFolder,
EventDispatcher $eventDispatcher
EventDispatcher $eventDispatcher,
View $view
) {
$this->logger = $logger;
$this->config = $config;
Expand All @@ -117,6 +123,7 @@ public function __construct(
$this->rootFolder = $rootFolder;
$this->sharingDisabledForUsersCache = new CappedMemoryCache();
$this->eventDispatcher = $eventDispatcher;
$this->view = $view;
}

/**
Expand Down Expand Up @@ -675,6 +682,93 @@ public function createShare(\OCP\Share\IShare $share) {
return $share;
}

/**
* Transfer shares from oldOwner to newOwner. Both old and new owners are uid
*
* finalTarget is of the form "user1/files/transferred from admin on 20180509"
*
* TransferShareException would be thrown when:
* - oldOwner, newOwner does not exist.
* - oldOwner and newOwner are same
* NotFoundException would be thrown when finalTarget does not exist in the file
* system
*
* @param IShare $share
* @param string $oldOwner
* @param string $newOwner
* @param string $finalTarget
* @throws TransferSharesException
* @throws NotFoundException
*/
public function transferShare(IShare $share, $oldOwner, $newOwner, $finalTarget) {
if ($this->userManager->get($oldOwner) === null) {
throw new TransferSharesException("The current owner of the share $oldOwner doesn't exist");
}
if ($this->userManager->get($newOwner) === null) {
throw new TransferSharesException("The future owner $newOwner, where the share has to be moved doesn't exist");
}

if ($oldOwner === $newOwner) {
throw new TransferSharesException("The current owner of the share and the future owner of the share are same");
}

//If the destination location, i.e finalTarget is not present, then
//throw an exception
if (!$this->view->file_exists($finalTarget)) {
throw new NotFoundException("The target location $finalTarget doesn't exist");
}

/**
* If the share was already shared with new owner, then we can delete it
*/
if ($share->getSharedWith() === $newOwner) {
// Unmount the shares before deleting, so we don't try to get the storage later on.
$shareMountPoint = $this->mountManager->find('/' . $newOwner . '/files' . $share->getTarget());
if ($shareMountPoint) {
$this->mountManager->removeMount($shareMountPoint->getMountPoint());
}
$this->deleteShare($share);
} else {
$sharedWith = $share->getSharedWith();

$targetFile = '/' . rtrim(basename($finalTarget), '/') . '/' . ltrim(basename($share->getTarget()), '/');
/**
* Scenario where share is made by old owner to a user different
* from new owner
*/
if (($sharedWith !== null) && ($sharedWith !== $oldOwner) && ($sharedWith !== $newOwner)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any chance to simplify this code block? It's a bit difficult to follow.
What happens is the "sharedBy" or "sharedOwner", just one of them not both, are the $oldOwner?

Copy link
Contributor Author

@sharidas sharidas May 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this means a folder/file shared to $oldOwner. That is incoming shares for $oldShare are not moved. Basically I have tried to move the changes from transfer ownership command to this method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For testing I created 4 users: admin, user1, user2 and user3.

  • login as admin and create a folder test_share and share to user3 and the table would look like this
    original
  • Now modify the table shown above as
    modified
  • Now run the transfer ownership command : owncloud3 git:(fix-public-link-transferownership) ✗ ./occ files:transfer-ownership admin user1 Cannot load Xdebug - it was already loaded Analysing files of admin ... 1 [============================] Collecting all share information for files and folder of admin ... 1 [============================] Transferring files to user1/files/transferred from admin on 20180508_064055 ... Restoring shares ... 1/1 [============================] 100% ➜ owncloud3 git:(fix-public-link-transferownership) ✗
  • The test_share is transferred to user1 and user3 can still see the share and the table becomes
    afterrunningcommand

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the next test I followed the same procedure in the above comment, I changed uid_initiator instead of uid_owner. And the result was the shares were transferred from admin to user1.

$sharedBy = $share->getSharedBy();
$sharedOwner = $share->getShareOwner();
//The origin of the share now has to be the destination user.
if ($sharedBy === $oldOwner) {
$share->setSharedBy($newOwner);
}
if ($sharedOwner === $oldOwner) {
$share->setShareOwner($newOwner);
}
if (($sharedBy === $oldOwner) || ($sharedOwner === $oldOwner)) {
$share->setTarget($targetFile);
}
} else {
if ($share->getShareOwner() === $oldOwner) {
$share->setShareOwner($newOwner);
}
if ($share->getSharedBy() === $oldOwner) {
$share->setSharedBy($newOwner);
}
}

/**
* Here we update the target when the share is link
*/
if ($share->getShareType() === \OCP\Share::SHARE_TYPE_LINK) {
$share->setTarget($targetFile);
}

$this->updateShare($share);
}
}

/**
* Update a share
*
Expand Down
43 changes: 43 additions & 0 deletions lib/public/Share/Exceptions/TransferSharesException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<?php
/**
* @author Sujith Haridasan <sharidasan@owncloud.com>
*
* @copyright Copyright (c) 2018, ownCloud GmbH
* @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 <http://www.gnu.org/licenses/>
*
*/
namespace OCP\Share\Exceptions;

/**
* Class TransferSharesException
*
* @package OCP\Share\Exceptions
* @since 10.0.9
*/
class TransferSharesException extends GenericShareException {

/**
* TransferSharesException constructor.
*
* @param string $message
* @param string $hint
* @param int $code
* @param \Exception|null $previous
* @since 10.0.9
*/
public function __construct($message = '', $hint = '', $code = 0, \Exception $previous = null) {
parent::__construct($message, $hint, $code, $previous);
}
}
24 changes: 24 additions & 0 deletions lib/public/Share/IManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,12 @@

namespace OCP\Share;

use OC\User\NoUserException;
use OCP\Files\Node;

use OCP\Files\NotFoundException;
use OCP\Share\Exceptions\ShareNotFound;
use OCP\Share\Exceptions\TransferSharesException;

/**
* Interface IManager
Expand Down Expand Up @@ -112,6 +115,27 @@ public function getAllSharesBy($userId, $shareTypes, $nodeIDs, $reshares = false
*/
public function getSharesBy($userId, $shareType, $path = null, $reshares = false, $limit = 50, $offset = 0);

/**
* Transfer shares from oldOwner to newOwner. Both old and new owners are uid
*
* @param IShare $share
* @param string $oldOwner - is the previous owner of the share, the uid string
* @param string $newOwner - is the new owner of the share, the uid string
* @param string $finalTarget - is the target folder where share has to be moved
*
* finalTarget is of the form "user1/files/transferred from admin on 20180509"
*
* TransferShareException would be thrown when:
* - oldOwner, newOwner does not exist.
* - oldOwner and newOwner are same
* NotFoundException would be thrown when finalTarget does not exist in the file
* system
*
* @throws TransferSharesException
* @throws NotFoundException
* @since 10.0.9
*/
public function transferShare(IShare $share, $oldOwner, $newOwner, $finalTarget);

/**
* Get shares shared with $userId for specified share types.
Expand Down
Loading