diff --git a/apps/files_sharing/api/ocssharewrapper.php b/apps/files_sharing/api/ocssharewrapper.php index 3ce2901dfb..ca04c656c2 100644 --- a/apps/files_sharing/api/ocssharewrapper.php +++ b/apps/files_sharing/api/ocssharewrapper.php @@ -26,25 +26,23 @@ class OCSShareWrapper { * @return Share20OCS */ private function getShare20OCS() { - return new Share20OCS(new \OC\Share20\Manager( - \OC::$server->getUserSession()->getUser(), - \OC::$server->getUserManager(), - \OC::$server->getGroupManager(), - \OC::$server->getLogger(), - \OC::$server->getAppConfig(), - \OC::$server->getUserFolder(), - new \OC\Share20\DefaultShareProvider( - \OC::$server->getDatabaseConnection(), - \OC::$server->getUserManager(), - \OC::$server->getGroupManager(), - \OC::$server->getUserFolder() - ) - ), - \OC::$server->getGroupManager(), - \OC::$server->getUserManager(), - \OC::$server->getRequest(), - \OC::$server->getUserFolder(), - \OC::$server->getURLGenerator()); + return new Share20OCS( + new \OC\Share20\Manager( + \OC::$server->getLogger(), + \OC::$server->getAppConfig(), + new \OC\Share20\DefaultShareProvider( + \OC::$server->getDatabaseConnection(), + \OC::$server->getUserManager(), + \OC::$server->getGroupManager(), + \OC::$server->getRootFolder() + ) + ), + \OC::$server->getGroupManager(), + \OC::$server->getUserManager(), + \OC::$server->getRequest(), + \OC::$server->getRootFolder(), + \OC::$server->getURLGenerator(), + \OC::$server->getUserSession()->getUser()); } public function getAllShares($params) { diff --git a/apps/files_sharing/api/share20ocs.php b/apps/files_sharing/api/share20ocs.php index aaf5a3c72b..6c25b4a442 100644 --- a/apps/files_sharing/api/share20ocs.php +++ b/apps/files_sharing/api/share20ocs.php @@ -22,35 +22,53 @@ namespace OCA\Files_Sharing\API; use OC\Share20\IShare; +use OCP\IGroupManager; +use OCP\IUserManager; +use OCP\IRequest; +use OCP\Files\Folder; +use OCP\IURLGenerator; +use OCP\IUser; +use OCP\Files\IRootFolder; + class Share20OCS { /** @var \OC\Share20\Manager */ private $shareManager; - /** @var \OCP\IGroupManager */ + /** @var IGroupManager */ private $groupManager; - /** @var \OCP\IUserManager */ + /** @var IUserManager */ private $userManager; - /** @var \OCP\IRequest */ + /** @var IRequest */ private $request; - /** @var \OCP\Files\Folder */ - private $userFolder; + /** @var IRootFolder */ + private $rootFolder; - public function __construct(\OC\Share20\Manager $shareManager, - \OCP\IGroupManager $groupManager, - \OCP\IUserManager $userManager, - \OCP\IRequest $request, - \OCP\Files\Folder $userFolder, - \OCP\IURLGenerator $urlGenerator) { + /** @var IUrlGenerator */ + private $urlGenerator; + + /** @var IUser */ + private $currentUser; + + public function __construct( + \OC\Share20\Manager $shareManager, + IGroupManager $groupManager, + IUserManager $userManager, + IRequest $request, + IRootFolder $rootFolder, + IURLGenerator $urlGenerator, + IUser $currentUser + ) { $this->shareManager = $shareManager; $this->userManager = $userManager; $this->groupManager = $groupManager; $this->request = $request; - $this->userFolder = $userFolder; + $this->rootFolder = $rootFolder; $this->urlGenerator = $urlGenerator; + $this->currentUser = $currentUser; } /** @@ -73,7 +91,7 @@ class Share20OCS { ]; $path = $share->getPath(); - $result['path'] = $this->userFolder->getRelativePath($path->getPath()); + $result['path'] = $this->rootFolder->getUserFolder($share->getShareOwner()->getUID())->getRelativePath($path->getPath()); if ($path instanceOf \OCP\Files\Folder) { $result['item_type'] = 'folder'; } else { @@ -131,8 +149,12 @@ class Share20OCS { return new \OC_OCS_Result(null, 404, 'wrong share ID, share doesn\'t exist.'); } - $share = $this->formatShare($share); - return new \OC_OCS_Result($share); + if ($this->canAccessShare($share)) { + $share = $this->formatShare($share); + return new \OC_OCS_Result($share); + } else { + return new \OC_OCS_Result(null, 404, 'wrong share ID, share doesn\'t exist.'); + } } /** @@ -156,6 +178,10 @@ class Share20OCS { \OCA\Files_Sharing\API\Local::deleteShare(['id' => $id]); } + if (!$this->canAccessShare($share)) { + return new \OC_OCS_Result(null, 404, 'could not delete share'); + } + try { $this->shareManager->deleteShare($share); } catch (\OC\Share20\Exception\BackendError $e) { @@ -164,4 +190,30 @@ class Share20OCS { return new \OC_OCS_Result(); } + + /** + * @param IShare $share + * @return bool + */ + protected function canAccessShare(IShare $share) { + // Owner of the file and the sharer of the file can always get share + if ($share->getShareOwner() === $this->currentUser || + $share->getSharedBy() === $this->currentUser + ) { + return true; + } + + // If the share is shared with you (or a group you are a member of) + if ($share->getShareType() === \OCP\Share::SHARE_TYPE_USER && + $share->getSharedWith() === $this->currentUser) { + return true; + } + + if ($share->getShareType() === \OCP\Share::SHARE_TYPE_GROUP && + $share->getSharedWith()->inGroup($this->currentUser)) { + return true; + } + + return false; + } } diff --git a/apps/files_sharing/tests/api/share20ocstest.php b/apps/files_sharing/tests/api/share20ocstest.php index f74585eb47..b7c56fe17f 100644 --- a/apps/files_sharing/tests/api/share20ocstest.php +++ b/apps/files_sharing/tests/api/share20ocstest.php @@ -20,29 +20,39 @@ */ namespace OCA\Files_Sharing\Tests\API; +use OC\Share20\IShare; use OCA\Files_Sharing\API\Share20OCS; +use OCP\IGroupManager; +use OCP\IUserManager; +use OCP\IRequest; +use OCP\IURLGenerator; +use OCP\IUser; +use OCP\Files\IRootFolder; class Share20OCSTest extends \Test\TestCase { - /** @var OC\Share20\Manager */ + /** @var \OC\Share20\Manager */ private $shareManager; - /** @var OCP\IGroupManager */ + /** @var IGroupManager */ private $groupManager; - /** @var OCP\IUserManager */ + /** @var IUserManager */ private $userManager; - /** @var OCP\IRequest */ + /** @var IRequest */ private $request; - /** @var OCP\Files\Folder */ - private $userFolder; + /** @var IRootFolder */ + private $rootFolder; - /** @var OCP\IURLGenerator */ + /** @var IURLGenerator */ private $urlGenerator; - /** @var OCS */ + /** @var IUser */ + private $currentUser; + + /** @var Share20OCS */ private $ocs; protected function setUp() { @@ -52,15 +62,19 @@ class Share20OCSTest extends \Test\TestCase { $this->groupManager = $this->getMock('OCP\IGroupManager'); $this->userManager = $this->getMock('OCP\IUserManager'); $this->request = $this->getMock('OCP\IRequest'); - $this->userFolder = $this->getMock('OCP\Files\Folder'); + $this->rootFolder = $this->getMock('OCP\Files\IRootFolder'); $this->urlGenerator = $this->getMock('OCP\IURLGenerator'); + $this->currentUser = $this->getMock('OCP\IUser'); - $this->ocs = new Share20OCS($this->shareManager, - $this->groupManager, - $this->userManager, - $this->request, - $this->userFolder, - $this->urlGenerator); + $this->ocs = new Share20OCS( + $this->shareManager, + $this->groupManager, + $this->userManager, + $this->request, + $this->rootFolder, + $this->urlGenerator, + $this->currentUser + ); } public function testDeleteShareShareNotFound() { @@ -76,6 +90,7 @@ class Share20OCSTest extends \Test\TestCase { public function testDeleteShareCouldNotDelete() { $share = $this->getMock('OC\Share20\IShare'); + $share->method('getShareOwner')->willReturn($this->currentUser); $this->shareManager ->expects($this->once()) ->method('getShareById') @@ -94,6 +109,7 @@ class Share20OCSTest extends \Test\TestCase { public function testDeleteShare() { $share = $this->getMock('OC\Share20\IShare'); + $share->method('getSharedBy')->willReturn($this->currentUser); $this->shareManager ->expects($this->once()) ->method('getShareById') @@ -119,7 +135,7 @@ class Share20OCSTest extends \Test\TestCase { $this->assertEquals($expected, $this->ocs->getShare(42)); } - public function createShare($id, $shareType, $sharedWith, $sharedBy, $path, $permissions, + public function createShare($id, $shareType, $sharedWith, $sharedBy, $shareOwner, $path, $permissions, $shareTime, $expiration, $parent, $target, $mail_send, $token=null, $password=null) { $share = $this->getMock('OC\Share20\IShare'); @@ -127,6 +143,7 @@ class Share20OCSTest extends \Test\TestCase { $share->method('getShareType')->willReturn($shareType); $share->method('getSharedWith')->willReturn($sharedWith); $share->method('getSharedBy')->willReturn($sharedBy); + $share->method('getShareOwner')->willReturn($shareOwner); $share->method('getPath')->willReturn($path); $share->method('getPermissions')->willReturn($permissions); $share->method('getShareTime')->willReturn($shareTime); @@ -173,17 +190,20 @@ class Share20OCSTest extends \Test\TestCase { $folder->method('getParent')->willReturn($parentFolder); // File shared with user - $share = $this->createShare(100, - \OCP\Share::SHARE_TYPE_USER, - $user, - $owner, - $file, - 4, - 5, - null, - 6, - 'target', - 0); + $share = $this->createShare( + 100, + \OCP\Share::SHARE_TYPE_USER, + $user, + $owner, + $owner, + $file, + 4, + 5, + null, + 6, + 'target', + 0 + ); $expected = [ 'id' => 100, 'share_type' => \OCP\Share::SHARE_TYPE_USER, @@ -209,17 +229,20 @@ class Share20OCSTest extends \Test\TestCase { $data[] = [$share, $expected]; // Folder shared with group - $share = $this->createShare(101, - \OCP\Share::SHARE_TYPE_GROUP, - $group, - $owner, - $folder, - 4, - 5, - null, - 6, - 'target', - 0); + $share = $this->createShare( + 101, + \OCP\Share::SHARE_TYPE_GROUP, + $group, + $owner, + $owner, + $folder, + 4, + 5, + null, + 6, + 'target', + 0 + ); $expected = [ 'id' => 101, 'share_type' => \OCP\Share::SHARE_TYPE_GROUP, @@ -244,57 +267,24 @@ class Share20OCSTest extends \Test\TestCase { ]; $data[] = [$share, $expected]; - // Folder shared with remote - $share = $this->createShare(101, - \OCP\Share::SHARE_TYPE_REMOTE, - 'user@remote.com', - $owner, - $folder, - 4, - 5, - null, - 6, - 'target', - 0); - $expected = [ - 'id' => 101, - 'share_type' => \OCP\Share::SHARE_TYPE_REMOTE, - 'share_with' => 'user@remote.com', - 'share_with_displayname' => 'user@remote.com', - 'uid_owner' => 'ownerId', - 'displayname_owner' => 'ownerDisplay', - 'item_type' => 'folder', - 'item_source' => 2, - 'file_source' => 2, - 'file_target' => 'target', - 'file_parent' => 3, - 'token' => null, - 'expiration' => null, - 'permissions' => 4, - 'stime' => 5, - 'parent' => 6, - 'storage_id' => 'STORAGE', - 'path' => 'folder', - 'storage' => null, // HACK around static function - 'mail_send' => 0, - ]; - $data[] = [$share, $expected]; - // File shared by link with Expire $expire = \DateTime::createFromFormat('Y-m-d h:i:s', '2000-01-02 01:02:03'); - $share = $this->createShare(101, - \OCP\Share::SHARE_TYPE_LINK, - null, - $owner, - $folder, - 4, - 5, - $expire, - 6, - 'target', - 0, - 'token', - 'password'); + $share = $this->createShare( + 101, + \OCP\Share::SHARE_TYPE_LINK, + null, + $owner, + $owner, + $folder, + 4, + 5, + $expire, + 6, + 'target', + 0, + 'token', + 'password' + ); $expected = [ 'id' => 101, 'share_type' => \OCP\Share::SHARE_TYPE_LINK, @@ -327,20 +317,78 @@ class Share20OCSTest extends \Test\TestCase { * @dataProvider dataGetShare */ public function testGetShare(\OC\Share20\IShare $share, array $result) { + $ocs = $this->getMockBuilder('OCA\Files_Sharing\API\Share20OCS') + ->setConstructorArgs([ + $this->shareManager, + $this->groupManager, + $this->userManager, + $this->request, + $this->rootFolder, + $this->urlGenerator, + $this->currentUser + ])->setMethods(['canAccessShare']) + ->getMock(); + + $ocs->method('canAccessShare')->willReturn(true); + $this->shareManager ->expects($this->once()) ->method('getShareById') ->with($share->getId()) ->willReturn($share); - $this->userFolder + $userFolder = $this->getMock('OCP\Files\Folder'); + $userFolder ->method('getRelativePath') ->will($this->returnArgument(0)); + $this->rootFolder->method('getUserFolder') + ->with($share->getShareOwner()->getUID()) + ->willReturn($userFolder); + $this->urlGenerator ->method('linkToRouteAbsolute') ->willReturn('url'); $expected = new \OC_OCS_Result($result); - $this->assertEquals($expected->getData(), $this->ocs->getShare($share->getId())->getData()); } + $this->assertEquals($expected->getData(), $ocs->getShare($share->getId())->getData()); + } + + public function testCanAccessShare() { + $share = $this->getMock('OC\Share20\IShare'); + $share->method('getShareOwner')->willReturn($this->currentUser); + $this->assertTrue($this->invokePrivate($this->ocs, 'canAccessShare', [$share])); + + $share = $this->getMock('OC\Share20\IShare'); + $share->method('getSharedBy')->willReturn($this->currentUser); + $this->assertTrue($this->invokePrivate($this->ocs, 'canAccessShare', [$share])); + + $share = $this->getMock('OC\Share20\IShare'); + $share->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_USER); + $share->method('getSharedWith')->willReturn($this->currentUser); + $this->assertTrue($this->invokePrivate($this->ocs, 'canAccessShare', [$share])); + + $share = $this->getMock('OC\Share20\IShare'); + $share->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_USER); + $share->method('getSharedWith')->willReturn($this->getMock('OCP\IUser')); + $this->assertFalse($this->invokePrivate($this->ocs, 'canAccessShare', [$share])); + + $share = $this->getMock('OC\Share20\IShare'); + $share->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_GROUP); + $group = $this->getMock('OCP\IGroup'); + $group->method('inGroup')->with($this->currentUser)->willReturn(true); + $share->method('getSharedWith')->willReturn($group); + $this->assertTrue($this->invokePrivate($this->ocs, 'canAccessShare', [$share])); + + $share = $this->getMock('OC\Share20\IShare'); + $share->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_GROUP); + $group = $this->getMock('OCP\IGroup'); + $group->method('inGroup')->with($this->currentUser)->willReturn(false); + $share->method('getSharedWith')->willReturn($group); + $this->assertFalse($this->invokePrivate($this->ocs, 'canAccessShare', [$share])); + + $share = $this->getMock('OC\Share20\IShare'); + $share->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_LINK); + $this->assertFalse($this->invokePrivate($this->ocs, 'canAccessShare', [$share])); + } } diff --git a/lib/private/share20/defaultshareprovider.php b/lib/private/share20/defaultshareprovider.php index 5805e41d41..bc3bc0ce9e 100644 --- a/lib/private/share20/defaultshareprovider.php +++ b/lib/private/share20/defaultshareprovider.php @@ -23,49 +23,61 @@ namespace OC\Share20; use OC\Share20\Exception\ShareNotFound; use OC\Share20\Exception\BackendError; use OCP\IUser; +use OCP\IGroupManager; +use OCP\IUserManager; +use OCP\Files\IRootFolder; +use OCP\IDBConnection; +use OCP\Files\Node; class DefaultShareProvider implements IShareProvider { - /** @var \OCP\IDBConnection */ + /** @var IDBConnection */ private $dbConn; - /** @var \OCP\IUserManager */ + /** @var IUserManager */ private $userManager; - /** @var \OCP\IGroupManager */ + /** @var IGroupManager */ private $groupManager; - /** @var \OCP\Files\Folder */ - private $userFolder; + /** @var IRootFolder */ + private $rootFolder; - public function __construct(\OCP\IDBConnection $connection, - \OCP\IUserManager $userManager, - \OCP\IGroupManager $groupManager, - \OCP\Files\Folder $userFolder) { + /** + * DefaultShareProvider constructor. + * + * @param IDBConnection $connection + * @param IUserManager $userManager + * @param IGroupManager $groupManager + * @param IRootFolder $rootFolder + */ + public function __construct( + IDBConnection $connection, + IUserManager $userManager, + IGroupManager $groupManager, + IRootFolder $rootFolder) { $this->dbConn = $connection; $this->userManager = $userManager; $this->groupManager = $groupManager; - $this->userFolder = $userFolder; + $this->rootFolder = $rootFolder; } /** * Share a path * - * @param Share $share - * @return Share The share object + * @param IShare $share + * @return IShare The share object */ - public function create(Share $share) { - throw new \Exception(); + public function create(IShare $share) { } /** * Update a share * - * @param Share $share - * @return Share The share object + * @param IShare $share + * @return IShare The share object */ - public function update(Share $share) { - throw new \Exception(); + public function update(IShare $share) { } /** @@ -125,7 +137,6 @@ class DefaultShareProvider implements IShareProvider { * @return Share[] */ public function getShares(IUser $user, $shareType, $offset, $limit) { - throw new \Exception(); } /** @@ -163,8 +174,7 @@ class DefaultShareProvider implements IShareProvider { * @param \OCP\Files\Node $path * @return IShare[] */ - public function getSharesByPath(\OCP\IUser $user, \OCP\Files\Node $path) { - throw new \Exception(); + public function getSharesByPath(IUser $user, Node $path) { } /** @@ -175,7 +185,6 @@ class DefaultShareProvider implements IShareProvider { * @param Share */ public function getSharedWithMe(IUser $user, $shareType = null) { - throw new \Exception(); } /** @@ -186,7 +195,6 @@ class DefaultShareProvider implements IShareProvider { * @param Share */ public function getShareByToken($token, $password = null) { - throw new \Exception(); } /** @@ -218,14 +226,14 @@ class DefaultShareProvider implements IShareProvider { $share->setSharedBy($this->userManager->get($data['uid_owner'])); // TODO: getById can return an array. How to handle this properly?? - $path = $this->userFolder->getById((int)$data['file_source']); - $path = $path[0]; - $share->setPath($path); + $folder = $this->rootFolder->getUserFolder($share->getSharedBy()->getUID()); + $path = $folder->getById((int)$data['file_source'])[0]; - $owner = $path->getStorage()->getOwner('.'); - if ($owner !== false) { - $share->setShareOwner($this->userManager->get($owner)); - } + $owner = $path->getOwner(); + $share->setShareOwner($owner); + + $path = $this->rootFolder->getUserFolder($owner->getUID())->getById((int)$data['file_source'])[0]; + $share->setPath($path); if ($data['expiration'] !== null) { $expiration = \DateTime::createFromFormat('Y-m-d H:i:s', $data['expiration']); @@ -235,5 +243,4 @@ class DefaultShareProvider implements IShareProvider { return $share; } - -} +} \ No newline at end of file diff --git a/lib/private/share20/ishare.php b/lib/private/share20/ishare.php index a80abebd71..2e54da7a02 100644 --- a/lib/private/share20/ishare.php +++ b/lib/private/share20/ishare.php @@ -38,7 +38,7 @@ interface IShare { /** * Set the path of this share * - * @param File|Folder $path + * @param Node $path * @return Share The modified object */ public function setPath(Node $path); diff --git a/lib/private/share20/ishareprovider.php b/lib/private/share20/ishareprovider.php index 833de1b58f..56a550acf7 100644 --- a/lib/private/share20/ishareprovider.php +++ b/lib/private/share20/ishareprovider.php @@ -29,18 +29,18 @@ interface IShareProvider { /** * Share a path * - * @param Share $share - * @return Share The share object + * @param IShare $share + * @return IShare The share object */ - public function create(Share $share); + public function create(IShare $share); /** * Update a share * - * @param Share $share - * @return Share The share object + * @param IShare $share + * @return IShare The share object */ - public function update(Share $share); + public function update(IShare $share); /** * Delete a share diff --git a/lib/private/share20/manager.php b/lib/private/share20/manager.php index 57d8496797..882b281c49 100644 --- a/lib/private/share20/manager.php +++ b/lib/private/share20/manager.php @@ -22,11 +22,7 @@ namespace OC\Share20; use OCP\IAppConfig; -use OCP\IUserManager; -use OCP\IGroupManager; -use OCP\IUser; use OCP\ILogger; -use OCP\Files\Folder; use OC\Share20\Exception\ShareNotFound; @@ -40,37 +36,26 @@ class Manager { */ private $defaultProvider; - /** @var IUser */ - private $currentUser; - - /** @var IUserManager */ - private $userManager; - - /** @var IGroupManager */ - private $groupManager; - /** @var ILogger */ private $logger; /** @var IAppConfig */ private $appConfig; - /** @var IFolder */ - private $userFolder; - - public function __construct(IUser $user, - IUserManager $userManager, - IGroupManager $groupManager, - ILogger $logger, - IAppConfig $appConfig, - Folder $userFolder, - IShareProvider $defaultProvider) { - $this->currentUser = $user; - $this->userManager = $userManager; - $this->groupManager = $groupManager; + /** + * Manager constructor. + * + * @param ILogger $logger + * @param IAppConfig $appConfig + * @param IShareProvider $defaultProvider + */ + public function __construct( + ILogger $logger, + IAppConfig $appConfig, + IShareProvider $defaultProvider + ) { $this->logger = $logger; $this->appConfig = $appConfig; - $this->userFolder = $userFolder; // TEMP SOLUTION JUST TO GET STARTED $this->defaultProvider = $defaultProvider; @@ -78,12 +63,11 @@ class Manager { /** * Share a path - * + * * @param Share $share * @return Share The share object */ public function createShare(Share $share) { - throw new \Exception(); } /** @@ -93,7 +77,6 @@ class Manager { * @return Share The share object */ public function updateShare(Share $share) { - throw new \Exception(); } /** @@ -118,7 +101,7 @@ class Manager { /** * Delete a share * - * @param Share $share + * @param IShare $share * @throws ShareNotFound * @throws \OC\Share20\Exception\BackendError */ @@ -126,7 +109,7 @@ class Manager { // Just to make sure we have all the info $share = $this->getShareById($share->getId()); - $formatHookParams = function($share) { + $formatHookParams = function(IShare $share) { // Prepare hook $shareType = $share->getShareType(); $sharedWith = ''; @@ -185,7 +168,6 @@ class Manager { * @return Share[] */ public function getShares($page=0, $perPage=50) { - throw new \Exception(); } /** @@ -203,12 +185,6 @@ class Manager { $share = $this->defaultProvider->getShareById($id); - if ($share->getSharedWith() !== $this->currentUser && - $share->getSharedBy() !== $this->currentUser && - $share->getShareOwner() !== $this->currentUser) { - throw new ShareNotFound(); - } - return $share; } @@ -222,7 +198,6 @@ class Manager { * @return Share[] */ public function getSharesByPath(\OCP\Files\Node $path, $page=0, $perPage=50) { - throw new \Exception(); } /** @@ -235,7 +210,6 @@ class Manager { * @return Share[] */ public function getSharedWithMe($shareType = null, $page=0, $perPage=50) { - throw new \Exception(); } /** @@ -246,10 +220,9 @@ class Manager { * * @return Share * - * @throws ShareNotFoundException + * @throws ShareNotFound */ public function getShareByToken($token, $password=null) { - throw new \Exception(); } /** @@ -277,6 +250,5 @@ class Manager { * @param \OCP\Files\Node $path */ public function getAccessList(\OCP\Files\Node $path) { - throw new \Exception(); } } diff --git a/lib/private/share20/share.php b/lib/private/share20/share.php index 4200816799..b7ce38ac61 100644 --- a/lib/private/share20/share.php +++ b/lib/private/share20/share.php @@ -58,7 +58,7 @@ class Share implements IShare { /** * Set the id of the share * - * @param int id + * @param string $id * @return Share The modified object */ public function setId($id) { @@ -292,7 +292,7 @@ class Share implements IShare { /** * Set the target of this share * - * @param string target + * @param string $target * @return Share The modified object */ public function setTarget($target) { diff --git a/tests/lib/share20/defaultshareprovidertest.php b/tests/lib/share20/defaultshareprovidertest.php index 4c0b552394..dc45bc7c08 100644 --- a/tests/lib/share20/defaultshareprovidertest.php +++ b/tests/lib/share20/defaultshareprovidertest.php @@ -23,7 +23,7 @@ namespace Test\Share20; use OCP\IDBConnection; use OCP\IUserManager; use OCP\IGroupManager; -use OCP\Files\Folder; +use OCP\Files\IRootFolder; use OC\Share20\DefaultShareProvider; class DefaultShareProviderTest extends \Test\TestCase { @@ -37,8 +37,8 @@ class DefaultShareProviderTest extends \Test\TestCase { /** @var IGroupManager */ protected $groupManager; - /** @var Folder */ - protected $userFolder; + /** @var IRootFolder */ + protected $rootFolder; /** @var DefaultShareProvider */ protected $provider; @@ -47,7 +47,7 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->dbConn = \OC::$server->getDatabaseConnection(); $this->userManager = $this->getMock('OCP\IUserManager'); $this->groupManager = $this->getMock('OCP\IGroupManager'); - $this->userFolder = $this->getMock('OCP\Files\Folder'); + $this->rootFolder = $this->getMock('OCP\Files\IRootFolder'); //Empty share table $this->dbConn->getQueryBuilder()->delete('share')->execute(); @@ -56,7 +56,7 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->dbConn, $this->userManager, $this->groupManager, - $this->userFolder + $this->rootFolder ); } @@ -65,7 +65,7 @@ class DefaultShareProviderTest extends \Test\TestCase { } /** - * @expectedException OC\Share20\Exception\ShareNotFound + * @expectedException \OC\Share20\Exception\ShareNotFound */ public function testGetShareByIdNotExist() { $this->provider->getShareById(1); @@ -97,25 +97,30 @@ class DefaultShareProviderTest extends \Test\TestCase { $id = $id['id']; $cursor->closeCursor(); - $storage = $this->getMock('OC\Files\Storage\Storage'); - $storage - ->expects($this->once()) - ->method('getOwner') - ->willReturn('shareOwner'); - $path = $this->getMock('OCP\Files\File'); - $path - ->expects($this->once()) - ->method('getStorage') - ->wilLReturn($storage); - $this->userFolder - ->expects($this->once()) - ->method('getById') - ->with(42) - ->willReturn([$path]); - $sharedWith = $this->getMock('OCP\IUser'); $sharedBy = $this->getMock('OCP\IUser'); + $sharedBy->method('getUID')->willReturn('sharedBy'); $shareOwner = $this->getMock('OCP\IUser'); + $shareOwner->method('getUID')->willReturn('shareOwner'); + + $sharedByPath = $this->getMock('\OCP\Files\File'); + $ownerPath = $this->getMock('\OCP\Files\File'); + + $sharedByPath->method('getOwner')->willReturn($shareOwner); + + $sharedByFolder = $this->getMock('\OCP\Files\Folder'); + $sharedByFolder->method('getById')->with(42)->willReturn([$sharedByPath]); + + $shareOwnerFolder = $this->getMock('\OCP\Files\Folder'); + $shareOwnerFolder->method('getById')->with(42)->willReturn([$ownerPath]); + + $this->rootFolder + ->method('getUserFolder') + ->will($this->returnValueMap([ + ['sharedBy', $sharedByFolder], + ['shareOwner', $shareOwnerFolder], + ])); + $this->userManager ->method('get') ->will($this->returnValueMap([ @@ -131,7 +136,7 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->assertEquals($sharedWith, $share->getSharedWith()); $this->assertEquals($sharedBy, $share->getSharedBy()); $this->assertEquals($shareOwner, $share->getShareOwner()); - $this->assertEquals($path, $share->getPath()); + $this->assertEquals($ownerPath, $share->getPath()); $this->assertEquals(13, $share->getPermissions()); $this->assertEquals(null, $share->getToken()); $this->assertEquals(null, $share->getExpirationDate()); @@ -164,25 +169,30 @@ class DefaultShareProviderTest extends \Test\TestCase { $id = $id['id']; $cursor->closeCursor(); - $storage = $this->getMock('OC\Files\Storage\Storage'); - $storage - ->expects($this->once()) - ->method('getOwner') - ->willReturn('shareOwner'); - $path = $this->getMock('OCP\Files\Folder'); - $path - ->expects($this->once()) - ->method('getStorage') - ->wilLReturn($storage); - $this->userFolder - ->expects($this->once()) - ->method('getById') - ->with(42) - ->willReturn([$path]); - $sharedWith = $this->getMock('OCP\IGroup'); $sharedBy = $this->getMock('OCP\IUser'); + $sharedBy->method('getUID')->willReturn('sharedBy'); $shareOwner = $this->getMock('OCP\IUser'); + $shareOwner->method('getUID')->willReturn('shareOwner'); + + $sharedByPath = $this->getMock('\OCP\Files\Folder'); + $ownerPath = $this->getMock('\OCP\Files\Folder'); + + $sharedByPath->method('getOwner')->willReturn($shareOwner); + + $sharedByFolder = $this->getMock('\OCP\Files\Folder'); + $sharedByFolder->method('getById')->with(42)->willReturn([$sharedByPath]); + + $shareOwnerFolder = $this->getMock('\OCP\Files\Folder'); + $shareOwnerFolder->method('getById')->with(42)->willReturn([$ownerPath]); + + $this->rootFolder + ->method('getUserFolder') + ->will($this->returnValueMap([ + ['sharedBy', $sharedByFolder], + ['shareOwner', $shareOwnerFolder], + ])); + $this->userManager ->method('get') ->will($this->returnValueMap([ @@ -202,7 +212,7 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->assertEquals($sharedWith, $share->getSharedWith()); $this->assertEquals($sharedBy, $share->getSharedBy()); $this->assertEquals($shareOwner, $share->getShareOwner()); - $this->assertEquals($path, $share->getPath()); + $this->assertEquals($ownerPath, $share->getPath()); $this->assertEquals(13, $share->getPermissions()); $this->assertEquals(null, $share->getToken()); $this->assertEquals(null, $share->getExpirationDate()); @@ -237,24 +247,29 @@ class DefaultShareProviderTest extends \Test\TestCase { $id = $id['id']; $cursor->closeCursor(); - $storage = $this->getMock('OC\Files\Storage\Storage'); - $storage - ->expects($this->once()) - ->method('getOwner') - ->willReturn('shareOwner'); - $path = $this->getMock('OCP\Files\Node'); - $path - ->expects($this->once()) - ->method('getStorage') - ->wilLReturn($storage); - $this->userFolder - ->expects($this->once()) - ->method('getById') - ->with(42) - ->willReturn([$path]); - $sharedBy = $this->getMock('OCP\IUser'); + $sharedBy->method('getUID')->willReturn('sharedBy'); $shareOwner = $this->getMock('OCP\IUser'); + $shareOwner->method('getUID')->willReturn('shareOwner'); + + $sharedByPath = $this->getMock('\OCP\Files\Folder'); + $ownerPath = $this->getMock('\OCP\Files\Folder'); + + $sharedByPath->method('getOwner')->willReturn($shareOwner); + + $sharedByFolder = $this->getMock('\OCP\Files\Folder'); + $sharedByFolder->method('getById')->with(42)->willReturn([$sharedByPath]); + + $shareOwnerFolder = $this->getMock('\OCP\Files\Folder'); + $shareOwnerFolder->method('getById')->with(42)->willReturn([$ownerPath]); + + $this->rootFolder + ->method('getUserFolder') + ->will($this->returnValueMap([ + ['sharedBy', $sharedByFolder], + ['shareOwner', $shareOwnerFolder], + ])); + $this->userManager ->method('get') ->will($this->returnValueMap([ @@ -269,79 +284,13 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->assertEquals('sharedWith', $share->getPassword()); $this->assertEquals($sharedBy, $share->getSharedBy()); $this->assertEquals($shareOwner, $share->getShareOwner()); - $this->assertEquals($path, $share->getPath()); + $this->assertEquals($ownerPath, $share->getPath()); $this->assertEquals(13, $share->getPermissions()); $this->assertEquals('token', $share->getToken()); $this->assertEquals(\DateTime::createFromFormat('Y-m-d H:i:s', '2000-01-02 00:00:00'), $share->getExpirationDate()); $this->assertEquals('myTarget', $share->getTarget()); } - public function testGetShareByIdRemoteShare() { - $qb = $this->dbConn->getQueryBuilder(); - - $qb->insert('share') - ->values([ - 'share_type' => $qb->expr()->literal(\OCP\Share::SHARE_TYPE_REMOTE), - 'share_with' => $qb->expr()->literal('sharedWith'), - 'uid_owner' => $qb->expr()->literal('sharedBy'), - 'item_type' => $qb->expr()->literal('file'), - 'file_source' => $qb->expr()->literal(42), - 'file_target' => $qb->expr()->literal('myTarget'), - 'permissions' => $qb->expr()->literal(13), - ]); - $this->assertEquals(1, $qb->execute()); - - // Get the id - $qb = $this->dbConn->getQueryBuilder(); - $cursor = $qb->select('id') - ->from('share') - ->setMaxResults(1) - ->orderBy('id', 'DESC') - ->execute(); - $id = $cursor->fetch(); - $id = $id['id']; - $cursor->closeCursor(); - - - $storage = $this->getMock('OC\Files\Storage\Storage'); - $storage - ->expects($this->once()) - ->method('getOwner') - ->willReturn('shareOwner'); - $path = $this->getMock('OCP\Files\Node'); - $path - ->expects($this->once()) - ->method('getStorage') - ->wilLReturn($storage); - $this->userFolder - ->expects($this->once()) - ->method('getById') - ->with(42) - ->willReturn([$path]); - - $sharedBy = $this->getMock('OCP\IUser'); - $shareOwner = $this->getMock('OCP\IUser'); - $this->userManager - ->method('get') - ->will($this->returnValueMap([ - ['sharedBy', $sharedBy], - ['shareOwner', $shareOwner], - ])); - - $share = $this->provider->getShareById($id); - - $this->assertEquals($id, $share->getId()); - $this->assertEquals(\OCP\Share::SHARE_TYPE_REMOTE, $share->getShareType()); - $this->assertEquals('sharedWith', $share->getSharedWith()); - $this->assertEquals($sharedBy, $share->getSharedBy()); - $this->assertEquals($shareOwner, $share->getShareOwner()); - $this->assertEquals($path, $share->getPath()); - $this->assertEquals(13, $share->getPermissions()); - $this->assertEquals(null, $share->getToken()); - $this->assertEquals(null, $share->getExpirationDate()); - $this->assertEquals('myTarget', $share->getTarget()); - } - public function testDeleteSingleShare() { $qb = $this->dbConn->getQueryBuilder(); $qb->insert('share') @@ -376,7 +325,7 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->dbConn, $this->userManager, $this->groupManager, - $this->userFolder, + $this->rootFolder, ] ) ->setMethods(['getShareById']) @@ -437,7 +386,7 @@ class DefaultShareProviderTest extends \Test\TestCase { $db, $this->userManager, $this->groupManager, - $this->userFolder, + $this->rootFolder, ] ) ->setMethods(['getShareById']) @@ -504,30 +453,45 @@ class DefaultShareProviderTest extends \Test\TestCase { ]); $qb->execute(); - - $storage = $this->getMock('OC\Files\Storage\Storage'); - $storage - ->method('getOwner') - ->willReturn('shareOwner'); - $path1 = $this->getMock('OCP\Files\File'); - $path1->expects($this->once())->method('getStorage')->willReturn($storage); - $path2 = $this->getMock('OCP\Files\Folder'); - $path2->expects($this->once())->method('getStorage')->willReturn($storage); - $this->userFolder - ->method('getById') - ->will($this->returnValueMap([ - [1, [$path1]], - [3, [$path2]], - ])); - $shareOwner = $this->getMock('OCP\IUser'); + $shareOwner->method('getUID')->willReturn('shareOwner'); $user1 = $this->getMock('OCP\IUser'); $user2 = $this->getMock('OCP\IUser'); + $user2->method('getUID')->willReturn('user2'); $user3 = $this->getMock('OCP\IUser'); + $user3->method('getUID')->willReturn('user3'); + + $user2Path = $this->getMock('\OCP\Files\File'); + $user2Path->expects($this->once())->method('getOwner')->willReturn($shareOwner); + $user2Folder = $this->getMock('\OCP\Files\Folder'); + $user2Folder->expects($this->once()) + ->method('getById') + ->with(1) + ->willReturn([$user2Path]); + + $user3Path = $this->getMock('\OCP\Files\Folder'); + $user3Path->expects($this->once())->method('getOwner')->willReturn($shareOwner); + $user3Folder = $this->getMock('\OCP\Files\Folder'); + $user3Folder->expects($this->once()) + ->method('getById') + ->with(3) + ->willReturn([$user3Path]); + + $ownerPath = $this->getMock('\OCP\Files\Folder'); + $ownerFolder = $this->getMock('\OCP\Files\Folder'); + $ownerFolder->method('getById')->willReturn([$ownerPath]); + + $this->rootFolder + ->method('getUserFolder') + ->will($this->returnValueMap([ + ['shareOwner', $ownerFolder], + ['user2', $user2Folder], + ['user3', $user3Folder], + ])); + $this->userManager ->method('get') ->will($this->returnValueMap([ - ['shareOwner', $shareOwner], ['user1', $user1], ['user2', $user2], ['user3', $user3], @@ -552,7 +516,7 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->assertEquals($user1, $children[0]->getSharedWith()); $this->assertEquals($user2, $children[0]->getSharedBy()); $this->assertEquals($shareOwner, $children[0]->getShareOwner()); - $this->assertEquals($path1, $children[0]->getPath()); + $this->assertEquals($ownerPath, $children[0]->getPath()); $this->assertEquals(2, $children[0]->getPermissions()); $this->assertEquals(null, $children[0]->getToken()); $this->assertEquals(null, $children[0]->getExpirationDate()); @@ -563,7 +527,7 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->assertEquals($group1, $children[1]->getSharedWith()); $this->assertEquals($user3, $children[1]->getSharedBy()); $this->assertEquals($shareOwner, $children[1]->getShareOwner()); - $this->assertEquals($path2, $children[1]->getPath()); + $this->assertEquals($ownerPath, $children[1]->getPath()); $this->assertEquals(4, $children[1]->getPermissions()); $this->assertEquals(null, $children[1]->getToken()); $this->assertEquals(null, $children[1]->getExpirationDate()); diff --git a/tests/lib/share20/managertest.php b/tests/lib/share20/managertest.php index 54be903356..e4d0bfad58 100644 --- a/tests/lib/share20/managertest.php +++ b/tests/lib/share20/managertest.php @@ -23,64 +23,39 @@ namespace Test\Share20; use OC\Share20\Manager; use OC\Share20\Exception; - -use OCP\IUser; -use OCP\IUserManager; -use OCP\IGroupManager; use OCP\ILogger; use OCP\IAppConfig; -use OCP\Files\Folder; -use OCP\Share20\IShareProvider; +use OC\Share20\IShareProvider; class ManagerTest extends \Test\TestCase { /** @var Manager */ protected $manager; - /** @var IUser */ - protected $user; - - /** @var IUserManager */ - protected $userManager; - - /** @var IGroupManager */ - protected $groupManager; - /** @var ILogger */ protected $logger; /** @var IAppConfig */ protected $appConfig; - /** @var Folder */ - protected $userFolder; - /** @var IShareProvider */ protected $defaultProvider; public function setUp() { - $this->user = $this->getMock('\OCP\IUser'); - $this->userManager = $this->getMock('\OCP\IUserManager'); - $this->groupManager = $this->getMock('\OCP\IGroupManager'); $this->logger = $this->getMock('\OCP\ILogger'); $this->appConfig = $this->getMock('\OCP\IAppConfig'); - $this->userFolder = $this->getMock('\OCP\Files\Folder'); $this->defaultProvider = $this->getMock('\OC\Share20\IShareProvider'); $this->manager = new Manager( - $this->user, - $this->userManager, - $this->groupManager, $this->logger, $this->appConfig, - $this->userFolder, $this->defaultProvider ); } /** - * @expectedException OC\Share20\Exception\ShareNotFound + * @expectedException \OC\Share20\Exception\ShareNotFound */ public function testDeleteNoShareId() { $share = $this->getMock('\OC\Share20\IShare'); @@ -115,12 +90,8 @@ class ManagerTest extends \Test\TestCase { public function testDelete($shareType, $sharedWith, $sharedWith_string) { $manager = $this->getMockBuilder('\OC\Share20\Manager') ->setConstructorArgs([ - $this->user, - $this->userManager, - $this->groupManager, $this->logger, $this->appConfig, - $this->userFolder, $this->defaultProvider ]) ->setMethods(['getShareById', 'deleteChildren']) @@ -205,12 +176,8 @@ class ManagerTest extends \Test\TestCase { public function testDeleteNested() { $manager = $this->getMockBuilder('\OC\Share20\Manager') ->setConstructorArgs([ - $this->user, - $this->userManager, - $this->groupManager, $this->logger, $this->appConfig, - $this->userFolder, $this->defaultProvider ]) ->setMethods(['getShareById']) @@ -349,12 +316,8 @@ class ManagerTest extends \Test\TestCase { public function testDeleteChildren() { $manager = $this->getMockBuilder('\OC\Share20\Manager') ->setConstructorArgs([ - $this->user, - $this->userManager, - $this->groupManager, $this->logger, $this->appConfig, - $this->userFolder, $this->defaultProvider ]) ->setMethods(['deleteShare']) @@ -391,82 +354,8 @@ class ManagerTest extends \Test\TestCase { $this->assertSame($shares, $result); } - /** - * @expectedException OC\Share20\Exception\ShareNotFound - */ - public function testGetShareByIdNotFoundInBackend() { - $this->defaultProvider - ->expects($this->once()) - ->method('getShareById') - ->with(42) - ->will($this->throwException(new \OC\Share20\Exception\ShareNotFound())); - - $this->manager->getShareById(42); - } - - /** - * @expectedException OC\Share20\Exception\ShareNotFound - */ - public function testGetShareByIdNotAuthorized() { - $otherUser1 = $this->getMock('\OCP\IUser'); - $otherUser2 = $this->getMock('\OCP\IUser'); - $otherUser3 = $this->getMock('\OCP\IUser'); - + public function testGetShareById() { $share = $this->getMock('\OC\Share20\IShare'); - $share - ->expects($this->once()) - ->method('getSharedWith') - ->with() - ->willReturn($otherUser1); - $share - ->expects($this->once()) - ->method('getSharedBy') - ->with() - ->willReturn($otherUser2); - $share - ->expects($this->once()) - ->method('getShareOwner') - ->with() - ->willReturn($otherUser3); - - $this->defaultProvider - ->expects($this->once()) - ->method('getShareById') - ->with(42) - ->willReturn($share); - - $this->manager->getShareById(42); - } - - public function dataGetShareById() { - return [ - ['getSharedWith'], - ['getSharedBy'], - ['getShareOwner'], - ]; - } - - /** - * @dataProvider dataGetShareById - */ - public function testGetShareById($currentUserIs) { - $otherUser1 = $this->getMock('\OCP\IUser'); - $otherUser2 = $this->getMock('\OCP\IUser'); - $otherUser3 = $this->getMock('\OCP\IUser'); - - $share = $this->getMock('\OC\Share20\IShare'); - $share - ->method('getSharedWith') - ->with() - ->willReturn($currentUserIs === 'getSharedWith' ? $this->user : $otherUser1); - $share - ->method('getSharedBy') - ->with() - ->willReturn($currentUserIs === 'getSharedBy' ? $this->user : $otherUser2); - $share - ->method('getShareOwner') - ->with() - ->willReturn($currentUserIs === 'getShareOwner' ? $this->user : $otherUser3); $this->defaultProvider ->expects($this->once())