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

Incomming federated shares are a special kind. We mount them as normal
webdav shares but we do supply owner info with the federated cloud id of
the share owner.

Since we do not yet have the new resharing behaviour on federated shares
we need to set the correct owner. Which will allow sharing and proper
mounting for other users.

fixes #22500

diff --git a/lib/private/share20/manager.php b/lib/private/share20/manager.php
index 4cff3dc818b33fb5be3d04c91fd44f7d34f575de..9b33e9475573e6306c03a0a92fa14558663718ef 100644 (file)
--- 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 fe94b72c4e68cfcde75934a3873a528070e2e658..c41f0754396dd4788ea9b65a484cfd064e957b92 100644 (file)
--- 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 */