Merge pull request #22503 from owncloud/issue_22500

When (re-)sharing an incomming federated share set the corrent owner

3 files changed, 105 insertions, 11 deletions

diff --git a/apps/files_versions/lib/storage.php b/apps/files_versions/lib/storage.php
index da736c868fc..4eac476ed66 100644
--- a/apps/files_versions/lib/storage.php
+++ b/apps/files_versions/lib/storage.php
@@ -104,6 +104,9 @@ class Storage {
 			$ownerView = new View('/'.$uid.'/files');
 			try {
 				$filename = $ownerView->getPath($info['fileid']);
+				// make sure that the file name doesn't end with a trailing slash
+				// can for example happen single files shared across servers
+				$filename = rtrim($filename, '/');
 			} catch (NotFoundException $e) {
 				$filename = null;
 			}
diff --git a/lib/private/share20/manager.php b/lib/private/share20/manager.php
index 4cff3dc818b..9b33e947557 100644
--- a/lib/private/share20/manager.php
+++ b/lib/private/share20/manager.php
@@ -517,8 +517,20 @@ class Manager implements IManager {
 		// Verify if there are any issues with the path
 		$this->pathCreateChecks($share->getNode());
 
-		// On creation of a share the owner is always the owner of the path
-		$share->setShareOwner($share->getNode()->getOwner()->getUID());
+		/*
+		 * On creation of a share the owner is always the owner of the path
+		 * Except for mounted federated shares.
+		 */
+		$storage = $share->getNode()->getStorage();
+		if ($storage->instanceOfStorage('OCA\Files_Sharing\External\Storage')) {
+			$parent = $share->getNode()->getParent();
+			while($parent->getStorage()->instanceOfStorage('OCA\Files_Sharing\External\Storage')) {
+				$parent = $parent->getParent();
+			}
+			$share->setShareOwner($parent->getOwner()->getUID());
+		} else {
+			$share->setShareOwner($share->getNode()->getOwner()->getUID());
+		}
 
 		// Cannot share with the owner
 		if ($share->getShareType() === \OCP\Share::SHARE_TYPE_USER &&
diff --git a/tests/lib/share20/managertest.php b/tests/lib/share20/managertest.php
index fe94b72c4e6..c41f0754396 100644
--- a/tests/lib/share20/managertest.php
+++ b/tests/lib/share20/managertest.php
@@ -1429,9 +1429,11 @@ class ManagerTest extends \Test\TestCase {
 		$shareOwner = $this->getMock('\OCP\IUser');
 		$shareOwner->method('getUID')->willReturn('shareOwner');
 
+		$storage = $this->getMock('\OCP\Files\Storage');
 		$path = $this->getMock('\OCP\Files\File');
 		$path->method('getOwner')->willReturn($shareOwner);
 		$path->method('getName')->willReturn('target');
+		$path->method('getStorage')->willReturn($storage);
 
 		$share = $this->createShare(
 			null,
@@ -1480,9 +1482,11 @@ class ManagerTest extends \Test\TestCase {
 		$shareOwner = $this->getMock('\OCP\IUser');
 		$shareOwner->method('getUID')->willReturn('shareOwner');
 
+		$storage = $this->getMock('\OCP\Files\Storage');
 		$path = $this->getMock('\OCP\Files\File');
 		$path->method('getOwner')->willReturn($shareOwner);
 		$path->method('getName')->willReturn('target');
+		$path->method('getStorage')->willReturn($storage);
 
 		$share = $this->createShare(
 			null,
@@ -1539,10 +1543,12 @@ class ManagerTest extends \Test\TestCase {
 		$shareOwner = $this->getMock('\OCP\IUser');
 		$shareOwner->method('getUID')->willReturn('shareOwner');
 
+		$storage = $this->getMock('\OCP\Files\Storage');
 		$path = $this->getMock('\OCP\Files\File');
 		$path->method('getOwner')->willReturn($shareOwner);
 		$path->method('getName')->willReturn('target');
 		$path->method('getId')->willReturn(1);
+		$path->method('getStorage')->willReturn($storage);
 
 		$date = new \DateTime();
 
@@ -1663,9 +1669,11 @@ class ManagerTest extends \Test\TestCase {
 		$shareOwner = $this->getMock('\OCP\IUser');
 		$shareOwner->method('getUID')->willReturn('shareOwner');
 
+		$storage = $this->getMock('\OCP\Files\Storage');
 		$path = $this->getMock('\OCP\Files\File');
 		$path->method('getOwner')->willReturn($shareOwner);
 		$path->method('getName')->willReturn('target');
+		$path->method('getStorage')->willReturn($storage);
 
 		$share = $this->createShare(
 			null,
@@ -1697,8 +1705,86 @@ class ManagerTest extends \Test\TestCase {
 			->method('setTarget')
 			->with('/target');
 
-		$dummy = new DummyCreate();
-		\OCP\Util::connectHook('OCP\Share', 'pre_shared', $dummy, 'listner');
+		$hookListner = $this->getMockBuilder('Dummy')->setMethods(['pre'])->getMock();
+		\OCP\Util::connectHook('OCP\Share', 'pre_shared', $hookListner, 'pre');
+		$hookListner->expects($this->once())
+			->method('pre')
+			->will($this->returnCallback(function (array $data) {
+				$data['run'] = false;
+				$data['error'] = 'I won\'t let you share!';
+			}));
+
+		$manager->createShare($share);
+	}
+
+	public function testCreateShareOfIncommingFederatedShare() {
+		$manager = $this->createManagerMock()
+			->setMethods(['canShare', 'generalCreateChecks', 'userCreateChecks', 'pathCreateChecks'])
+			->getMock();
+
+		$shareOwner = $this->getMock('\OCP\IUser');
+		$shareOwner->method('getUID')->willReturn('shareOwner');
+
+		$storage = $this->getMock('\OCP\Files\Storage');
+		$storage->method('instanceOfStorage')
+			->with('OCA\Files_Sharing\External\Storage')
+			->willReturn(true);
+
+		$storage2 = $this->getMock('\OCP\Files\Storage');
+		$storage2->method('instanceOfStorage')
+			->with('OCA\Files_Sharing\External\Storage')
+			->willReturn(false);
+
+		$path = $this->getMock('\OCP\Files\File');
+		$path->expects($this->never())->method('getOwner');
+		$path->method('getName')->willReturn('target');
+		$path->method('getStorage')->willReturn($storage);
+
+		$parent = $this->getMock('\OCP\Files\Folder');
+		$parent->method('getStorage')->willReturn($storage);
+
+		$parentParent = $this->getMock('\OCP\Files\Folder');
+		$parentParent->method('getStorage')->willReturn($storage2);
+		$parentParent->method('getOwner')->willReturn($shareOwner);
+
+		$path->method('getParent')->willReturn($parent);
+		$parent->method('getParent')->willReturn($parentParent);
+
+		$share = $this->createShare(
+			null,
+			\OCP\Share::SHARE_TYPE_USER,
+			$path,
+			'sharedWith',
+			'sharedBy',
+			null,
+			\OCP\Constants::PERMISSION_ALL);
+
+		$manager->expects($this->once())
+			->method('canShare')
+			->with($share)
+			->willReturn(true);
+		$manager->expects($this->once())
+			->method('generalCreateChecks')
+			->with($share);;
+		$manager->expects($this->once())
+			->method('userCreateChecks')
+			->with($share);;
+		$manager->expects($this->once())
+			->method('pathCreateChecks')
+			->with($path);
+
+		$this->defaultProvider
+			->expects($this->once())
+			->method('create')
+			->with($share)
+			->will($this->returnArgument(0));
+
+		$share->expects($this->once())
+			->method('setShareOwner')
+			->with('shareOwner');
+		$share->expects($this->once())
+			->method('setTarget')
+			->with('/target');
 
 		$manager->createShare($share);
 	}
@@ -2269,13 +2355,6 @@ class DummyPassword {
 	}
 }
 
-class DummyCreate {
-	public function listner($array) {
-		$array['run'] = false;
-		$array['error'] = 'I won\'t let you share!';
-	}
-}
-
 class DummyFactory implements IProviderFactory {
 
 	/** @var IShareProvider */