Merge pull request #20691 from owncloud/share2.0_di_fixes

[Sharing 2.0] di fixes

3 files changed, 222 insertions, 124 deletions

diff --git a/apps/files_sharing/api/ocssharewrapper.php b/apps/files_sharing/api/ocssharewrapper.php
index 3ce2901dfb4..ca04c656c28 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 aaf5a3c72b6..6c25b4a4426 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;
+
+	/** @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) {
+	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 f74585eb47d..b7c56fe17f6 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->ocs = new Share20OCS($this->shareManager,
-									$this->groupManager,
-									$this->userManager,
-									$this->request,
-									$this->userFolder,
-									$this->urlGenerator);
+		$this->currentUser = $this->getMock('OCP\IUser');
+
+		$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]));
+	}
 }