From aeae73b364048ebf3baf8cf6d692ac4f62467e7e Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Mon, 23 Nov 2015 14:06:25 +0100 Subject: [PATCH 1/5] [Sharing 2.0] Removed unused DI stuff The share manager etc should not care about filtering stuff. They should return what is asked for them. --- apps/files_sharing/api/ocssharewrapper.php | 35 +++--- .../tests/api/share20ocstest.php | 14 +-- lib/private/share20/manager.php | 44 ++----- tests/lib/share20/managertest.php | 117 +----------------- 4 files changed, 34 insertions(+), 176 deletions(-) diff --git a/apps/files_sharing/api/ocssharewrapper.php b/apps/files_sharing/api/ocssharewrapper.php index 3ce2901dfb..b93a994f75 100644 --- a/apps/files_sharing/api/ocssharewrapper.php +++ b/apps/files_sharing/api/ocssharewrapper.php @@ -26,25 +26,22 @@ 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->getUserFolder() + ) + ), + \OC::$server->getGroupManager(), + \OC::$server->getUserManager(), + \OC::$server->getRequest(), + \OC::$server->getUserFolder(), + \OC::$server->getURLGenerator()); } public function getAllShares($params) { diff --git a/apps/files_sharing/tests/api/share20ocstest.php b/apps/files_sharing/tests/api/share20ocstest.php index f74585eb47..9393b8d12c 100644 --- a/apps/files_sharing/tests/api/share20ocstest.php +++ b/apps/files_sharing/tests/api/share20ocstest.php @@ -24,25 +24,25 @@ use OCA\Files_Sharing\API\Share20OCS; class Share20OCSTest extends \Test\TestCase { - /** @var OC\Share20\Manager */ + /** @var \OC\Share20\Manager */ private $shareManager; - /** @var OCP\IGroupManager */ + /** @var \OCP\IGroupManager */ private $groupManager; - /** @var OCP\IUserManager */ + /** @var \OCP\IUserManager */ private $userManager; - /** @var OCP\IRequest */ + /** @var \OCP\IRequest */ private $request; - /** @var OCP\Files\Folder */ + /** @var \OCP\Files\Folder */ private $userFolder; - /** @var OCP\IURLGenerator */ + /** @var \OCP\IURLGenerator */ private $urlGenerator; - /** @var OCS */ + /** @var Share20OCS */ private $ocs; protected function setUp() { diff --git a/lib/private/share20/manager.php b/lib/private/share20/manager.php index 57d8496797..e58110b40d 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,19 @@ 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; + 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; @@ -118,7 +96,7 @@ class Manager { /** * Delete a share * - * @param Share $share + * @param IShare $share * @throws ShareNotFound * @throws \OC\Share20\Exception\BackendError */ @@ -126,7 +104,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 = ''; @@ -203,12 +181,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; } @@ -246,7 +218,7 @@ class Manager { * * @return Share * - * @throws ShareNotFoundException + * @throws ShareNotFound */ public function getShareByToken($token, $password=null) { throw new \Exception(); 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()) From a2b8483779b5cb868309ca3f98051bfcaafd6ff9 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Mon, 23 Nov 2015 17:10:58 +0100 Subject: [PATCH 2/5] [Sharing 2.0] Default share provider only generic DI No injection of userfolders etc. Only generic DI components (IRootFolder) etc should be used to make sure we can also run this from the cli --- apps/files_sharing/api/ocssharewrapper.php | 2 +- lib/private/share20/defaultshareprovider.php | 22 +- .../lib/share20/defaultshareprovidertest.php | 252 ++++++++---------- 3 files changed, 120 insertions(+), 156 deletions(-) diff --git a/apps/files_sharing/api/ocssharewrapper.php b/apps/files_sharing/api/ocssharewrapper.php index b93a994f75..ab54e5e5e3 100644 --- a/apps/files_sharing/api/ocssharewrapper.php +++ b/apps/files_sharing/api/ocssharewrapper.php @@ -34,7 +34,7 @@ class OCSShareWrapper { \OC::$server->getDatabaseConnection(), \OC::$server->getUserManager(), \OC::$server->getGroupManager(), - \OC::$server->getUserFolder() + \OC::$server->getRootFolder() ) ), \OC::$server->getGroupManager(), diff --git a/lib/private/share20/defaultshareprovider.php b/lib/private/share20/defaultshareprovider.php index 5805e41d41..15add93ce6 100644 --- a/lib/private/share20/defaultshareprovider.php +++ b/lib/private/share20/defaultshareprovider.php @@ -35,17 +35,17 @@ class DefaultShareProvider implements IShareProvider { /** @var \OCP\IGroupManager */ private $groupManager; - /** @var \OCP\Files\Folder */ - private $userFolder; + /** @var \OCP\Files\IRootFolder */ + private $rootFolder; public function __construct(\OCP\IDBConnection $connection, \OCP\IUserManager $userManager, \OCP\IGroupManager $groupManager, - \OCP\Files\Folder $userFolder) { + \OCP\Files\IRootFolder $rootFolder) { $this->dbConn = $connection; $this->userManager = $userManager; $this->groupManager = $groupManager; - $this->userFolder = $userFolder; + $this->rootFolder = $rootFolder; } /** @@ -218,14 +218,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']); 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()); From ab02b5c36e949c17433d11b3b5ca082739c42732 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 24 Nov 2015 09:37:17 +0100 Subject: [PATCH 3/5] [Sharing 2.0] Move authentication to the OCS API --- apps/files_sharing/api/ocssharewrapper.php | 3 +- apps/files_sharing/api/share20ocs.php | 75 ++++++++-- .../tests/api/share20ocstest.php | 129 +++++++++++------- 3 files changed, 146 insertions(+), 61 deletions(-) diff --git a/apps/files_sharing/api/ocssharewrapper.php b/apps/files_sharing/api/ocssharewrapper.php index ab54e5e5e3..2a2c16da1f 100644 --- a/apps/files_sharing/api/ocssharewrapper.php +++ b/apps/files_sharing/api/ocssharewrapper.php @@ -41,7 +41,8 @@ class OCSShareWrapper { \OC::$server->getUserManager(), \OC::$server->getRequest(), \OC::$server->getUserFolder(), - \OC::$server->getURLGenerator()); + \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..788cbe8586 100644 --- a/apps/files_sharing/api/share20ocs.php +++ b/apps/files_sharing/api/share20ocs.php @@ -22,35 +22,52 @@ 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; + 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 */ + /** @var Folder */ private $userFolder; - 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, + \OCP\IGroupManager $groupManager, + \OCP\IUserManager $userManager, + \OCP\IRequest $request, + \OCP\Files\Folder $userFolder, + \OCP\IURLGenerator $urlGenerator, + \OCP\IUser $currentUser + ) { $this->shareManager = $shareManager; $this->userManager = $userManager; $this->groupManager = $groupManager; $this->request = $request; $this->userFolder = $userFolder; $this->urlGenerator = $urlGenerator; + $this->currentUser = $currentUser; } /** @@ -131,8 +148,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 +177,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 +189,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 9393b8d12c..fb37824b33 100644 --- a/apps/files_sharing/tests/api/share20ocstest.php +++ b/apps/files_sharing/tests/api/share20ocstest.php @@ -20,28 +20,38 @@ */ 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\Files\Folder; +use OCP\IURLGenerator; +use OCP\IUser; class Share20OCSTest extends \Test\TestCase { /** @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 */ + /** @var Folder */ private $userFolder; - /** @var \OCP\IURLGenerator */ + /** @var IURLGenerator */ private $urlGenerator; + /** @var IUser */ + private $currentUser; + /** @var Share20OCS */ private $ocs; @@ -54,13 +64,17 @@ class Share20OCSTest extends \Test\TestCase { $this->request = $this->getMock('OCP\IRequest'); $this->userFolder = $this->getMock('OCP\Files\Folder'); $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->userFolder, + $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') @@ -244,42 +260,6 @@ 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, @@ -327,6 +307,20 @@ 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->userFolder, + $this->urlGenerator, + $this->currentUser + ])->setMethods(['canAccessShare']) + ->getMock(); + + $ocs->method('canAccessShare')->willReturn(true); + $this->shareManager ->expects($this->once()) ->method('getShareById') @@ -342,5 +336,44 @@ class Share20OCSTest extends \Test\TestCase { ->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])); + } } From 85976b72937eae0e99d974551baf3aa96fa8d041 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 24 Nov 2015 09:58:37 +0100 Subject: [PATCH 4/5] [Sharing 2.0] Fix phpdoc etc --- apps/files_sharing/api/share20ocs.php | 12 ++--- lib/private/share20/defaultshareprovider.php | 53 +++++++++++--------- lib/private/share20/ishare.php | 2 +- lib/private/share20/ishareprovider.php | 12 ++--- lib/private/share20/manager.php | 16 +++--- lib/private/share20/share.php | 4 +- 6 files changed, 53 insertions(+), 46 deletions(-) diff --git a/apps/files_sharing/api/share20ocs.php b/apps/files_sharing/api/share20ocs.php index 788cbe8586..1f27168c70 100644 --- a/apps/files_sharing/api/share20ocs.php +++ b/apps/files_sharing/api/share20ocs.php @@ -54,12 +54,12 @@ class Share20OCS { public function __construct( \OC\Share20\Manager $shareManager, - \OCP\IGroupManager $groupManager, - \OCP\IUserManager $userManager, - \OCP\IRequest $request, - \OCP\Files\Folder $userFolder, - \OCP\IURLGenerator $urlGenerator, - \OCP\IUser $currentUser + IGroupManager $groupManager, + IUserManager $userManager, + IRequest $request, + Folder $userFolder, + IURLGenerator $urlGenerator, + IUser $currentUser ) { $this->shareManager = $shareManager; $this->userManager = $userManager; diff --git a/lib/private/share20/defaultshareprovider.php b/lib/private/share20/defaultshareprovider.php index 15add93ce6..bc3bc0ce9e 100644 --- a/lib/private/share20/defaultshareprovider.php +++ b/lib/private/share20/defaultshareprovider.php @@ -23,25 +23,39 @@ 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\IRootFolder */ + /** @var IRootFolder */ private $rootFolder; - public function __construct(\OCP\IDBConnection $connection, - \OCP\IUserManager $userManager, - \OCP\IGroupManager $groupManager, - \OCP\Files\IRootFolder $rootFolder) { + /** + * 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; @@ -51,21 +65,19 @@ class DefaultShareProvider implements 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) { - 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(); } /** @@ -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 e58110b40d..882b281c49 100644 --- a/lib/private/share20/manager.php +++ b/lib/private/share20/manager.php @@ -42,6 +42,13 @@ class Manager { /** @var IAppConfig */ private $appConfig; + /** + * Manager constructor. + * + * @param ILogger $logger + * @param IAppConfig $appConfig + * @param IShareProvider $defaultProvider + */ public function __construct( ILogger $logger, IAppConfig $appConfig, @@ -56,12 +63,11 @@ class Manager { /** * Share a path - * + * * @param Share $share * @return Share The share object */ public function createShare(Share $share) { - throw new \Exception(); } /** @@ -71,7 +77,6 @@ class Manager { * @return Share The share object */ public function updateShare(Share $share) { - throw new \Exception(); } /** @@ -163,7 +168,6 @@ class Manager { * @return Share[] */ public function getShares($page=0, $perPage=50) { - throw new \Exception(); } /** @@ -194,7 +198,6 @@ class Manager { * @return Share[] */ public function getSharesByPath(\OCP\Files\Node $path, $page=0, $perPage=50) { - throw new \Exception(); } /** @@ -207,7 +210,6 @@ class Manager { * @return Share[] */ public function getSharedWithMe($shareType = null, $page=0, $perPage=50) { - throw new \Exception(); } /** @@ -221,7 +223,6 @@ class Manager { * @throws ShareNotFound */ public function getShareByToken($token, $password=null) { - throw new \Exception(); } /** @@ -249,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) { From 8d309767d7a48a7fe1dd23fd79bc827c29f94931 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 24 Nov 2015 10:16:02 +0100 Subject: [PATCH 5/5] [Sharing 2.0] Use the rootfolder to get the path of a share We need to use the rootfolder here since we also other people than the shareOwner can request a share. --- apps/files_sharing/api/ocssharewrapper.php | 2 +- apps/files_sharing/api/share20ocs.php | 11 +- .../tests/api/share20ocstest.php | 101 ++++++++++-------- 3 files changed, 65 insertions(+), 49 deletions(-) diff --git a/apps/files_sharing/api/ocssharewrapper.php b/apps/files_sharing/api/ocssharewrapper.php index 2a2c16da1f..ca04c656c2 100644 --- a/apps/files_sharing/api/ocssharewrapper.php +++ b/apps/files_sharing/api/ocssharewrapper.php @@ -40,7 +40,7 @@ class OCSShareWrapper { \OC::$server->getGroupManager(), \OC::$server->getUserManager(), \OC::$server->getRequest(), - \OC::$server->getUserFolder(), + \OC::$server->getRootFolder(), \OC::$server->getURLGenerator(), \OC::$server->getUserSession()->getUser()); } diff --git a/apps/files_sharing/api/share20ocs.php b/apps/files_sharing/api/share20ocs.php index 1f27168c70..6c25b4a442 100644 --- a/apps/files_sharing/api/share20ocs.php +++ b/apps/files_sharing/api/share20ocs.php @@ -28,6 +28,7 @@ use OCP\IRequest; use OCP\Files\Folder; use OCP\IURLGenerator; use OCP\IUser; +use OCP\Files\IRootFolder; class Share20OCS { @@ -43,8 +44,8 @@ class Share20OCS { /** @var IRequest */ private $request; - /** @var Folder */ - private $userFolder; + /** @var IRootFolder */ + private $rootFolder; /** @var IUrlGenerator */ private $urlGenerator; @@ -57,7 +58,7 @@ class Share20OCS { IGroupManager $groupManager, IUserManager $userManager, IRequest $request, - Folder $userFolder, + IRootFolder $rootFolder, IURLGenerator $urlGenerator, IUser $currentUser ) { @@ -65,7 +66,7 @@ class Share20OCS { $this->userManager = $userManager; $this->groupManager = $groupManager; $this->request = $request; - $this->userFolder = $userFolder; + $this->rootFolder = $rootFolder; $this->urlGenerator = $urlGenerator; $this->currentUser = $currentUser; } @@ -90,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 { diff --git a/apps/files_sharing/tests/api/share20ocstest.php b/apps/files_sharing/tests/api/share20ocstest.php index fb37824b33..b7c56fe17f 100644 --- a/apps/files_sharing/tests/api/share20ocstest.php +++ b/apps/files_sharing/tests/api/share20ocstest.php @@ -25,9 +25,9 @@ use OCA\Files_Sharing\API\Share20OCS; use OCP\IGroupManager; use OCP\IUserManager; use OCP\IRequest; -use OCP\Files\Folder; use OCP\IURLGenerator; use OCP\IUser; +use OCP\Files\IRootFolder; class Share20OCSTest extends \Test\TestCase { @@ -43,8 +43,8 @@ class Share20OCSTest extends \Test\TestCase { /** @var IRequest */ private $request; - /** @var Folder */ - private $userFolder; + /** @var IRootFolder */ + private $rootFolder; /** @var IURLGenerator */ private $urlGenerator; @@ -62,7 +62,7 @@ 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'); @@ -71,7 +71,7 @@ class Share20OCSTest extends \Test\TestCase { $this->groupManager, $this->userManager, $this->request, - $this->userFolder, + $this->rootFolder, $this->urlGenerator, $this->currentUser ); @@ -135,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'); @@ -143,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); @@ -189,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, @@ -225,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, @@ -262,19 +269,22 @@ class Share20OCSTest extends \Test\TestCase { // 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, @@ -313,7 +323,7 @@ class Share20OCSTest extends \Test\TestCase { $this->groupManager, $this->userManager, $this->request, - $this->userFolder, + $this->rootFolder, $this->urlGenerator, $this->currentUser ])->setMethods(['canAccessShare']) @@ -327,10 +337,15 @@ class Share20OCSTest extends \Test\TestCase { ->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');