Merge pull request #22055 from owncloud/systemtags-checkfileidowner

Make sure user has access to file for system tag operations

6 files changed, 109 insertions, 13 deletions

diff --git a/apps/dav/lib/rootcollection.php b/apps/dav/lib/rootcollection.php
index b7c2995dbad..5be469e7b0c 100644
--- a/apps/dav/lib/rootcollection.php
+++ b/apps/dav/lib/rootcollection.php
@@ -68,7 +68,8 @@ class RootCollection extends SimpleCollection {
 			\OC::$server->getSystemTagManager(),
 			\OC::$server->getSystemTagObjectMapper(),
 			\OC::$server->getUserSession(),
-			\OC::$server->getGroupManager()
+			\OC::$server->getGroupManager(),
+			\OC::$server->getRootFolder()
 		);
 		$commentsCollection = new Comments\RootCollection(
 			\OC::$server->getCommentsManager(),
diff --git a/apps/dav/lib/systemtag/systemtagplugin.php b/apps/dav/lib/systemtag/systemtagplugin.php
index 4636ed428b1..3348b431c47 100644
--- a/apps/dav/lib/systemtag/systemtagplugin.php
+++ b/apps/dav/lib/systemtag/systemtagplugin.php
@@ -104,12 +104,7 @@ class SystemTagPlugin extends \Sabre\DAV\ServerPlugin {
 		$path = $request->getPath();
 
 		// Making sure the node exists
-		try {
-			$node = $this->server->tree->getNodeForPath($path);
-		} catch (NotFound $e) {
-			return null;
-		}
-
+		$node = $this->server->tree->getNodeForPath($path);
 		if ($node instanceof SystemTagsByIdCollection || $node instanceof SystemTagsObjectMappingCollection) {
 			$data = $request->getBodyAsString();
 
diff --git a/apps/dav/lib/systemtag/systemtagsobjecttypecollection.php b/apps/dav/lib/systemtag/systemtagsobjecttypecollection.php
index 166e3219bc1..93cc7561928 100644
--- a/apps/dav/lib/systemtag/systemtagsobjecttypecollection.php
+++ b/apps/dav/lib/systemtag/systemtagsobjecttypecollection.php
@@ -25,12 +25,14 @@ namespace OCA\DAV\SystemTag;
 
 use Sabre\DAV\Exception\Forbidden;
 use Sabre\DAV\Exception\MethodNotAllowed;
+use Sabre\DAV\Exception\NotFound;
 use Sabre\DAV\ICollection;
 
 use OCP\SystemTag\ISystemTagManager;
 use OCP\SystemTag\ISystemTagObjectMapper;
 use OCP\IUserSession;
 use OCP\IGroupManager;
+use OCP\Files\IRootFolder;
 
 /**
  * Collection containing object ids by object type
@@ -63,6 +65,11 @@ class SystemTagsObjectTypeCollection implements ICollection {
 	private $userSession;
 
 	/**
+	 * @var IRootFolder
+	 **/
+	protected $fileRoot;
+
+	/**
 	 * Constructor
 	 *
 	 * @param string $objectType object type
@@ -70,19 +77,22 @@ class SystemTagsObjectTypeCollection implements ICollection {
 	 * @param ISystemTagObjectMapper $tagMapper
 	 * @param IUserSession $userSession
 	 * @param IGroupManager $groupManager
+	 * @param IRootFolder $fileRoot
 	 */
 	public function __construct(
 		$objectType, 
 		ISystemTagManager $tagManager,
 		ISystemTagObjectMapper $tagMapper,
 		IUserSession $userSession,
-		IGroupManager $groupManager
+		IGroupManager $groupManager,
+		IRootFolder $fileRoot
 	) {
 		$this->tagManager = $tagManager;
 		$this->tagMapper = $tagMapper;
 		$this->objectType = $objectType;
 		$this->userSession = $userSession;
 		$this->groupManager = $groupManager;
+		$this->fileRoot = $fileRoot;
 	}
 
 	/**
@@ -116,6 +126,10 @@ class SystemTagsObjectTypeCollection implements ICollection {
 	 * @param string $objectId
 	 */
 	function getChild($objectId) {
+		// make sure the object exists and is reachable
+		if(!$this->childExists($objectId)) {
+			throw new NotFound('Entity does not exist or is not available');
+		}
 		return new SystemTagsObjectMappingCollection(
 			$objectId,
 			$this->objectType,
@@ -134,6 +148,13 @@ class SystemTagsObjectTypeCollection implements ICollection {
 	 * @param string $name
 	 */
 	function childExists($name) {
+		// TODO: make this more abstract
+		if ($this->objectType === 'files') {
+			// make sure the object is reachable for the current user
+			$userId = $this->userSession->getUser()->getUID();
+			$nodes = $this->fileRoot->getUserFolder($userId)->getById(intval($name));
+			return !empty($nodes);
+		}
 		return true;
 	}
 
diff --git a/apps/dav/lib/systemtag/systemtagsrelationscollection.php b/apps/dav/lib/systemtag/systemtagsrelationscollection.php
index 223aaac93d6..6af4edfc1ab 100644
--- a/apps/dav/lib/systemtag/systemtagsrelationscollection.php
+++ b/apps/dav/lib/systemtag/systemtagsrelationscollection.php
@@ -28,6 +28,7 @@ use Sabre\DAV\Exception\Forbidden;
 use Sabre\DAV\SimpleCollection;
 use OCP\IUserSession;
 use OCP\IGroupManager;
+use OCP\Files\IRootFolder;
 
 class SystemTagsRelationsCollection extends SimpleCollection {
 
@@ -38,12 +39,14 @@ class SystemTagsRelationsCollection extends SimpleCollection {
 	 * @param ISystemTagObjectMapper $tagMapper
 	 * @param IUserSession $userSession
 	 * @param IGroupManager $groupManager
+	 * @param IRootFolder $fileRoot
 	 */
 	public function __construct(
 		ISystemTagManager $tagManager,
 		ISystemTagObjectMapper $tagMapper,
 		IUserSession $userSession,
-		IGroupManager $groupManager
+		IGroupManager $groupManager,
+		IRootFolder $fileRoot
 	) {
 		$children = [
 			new SystemTagsObjectTypeCollection(
@@ -51,7 +54,8 @@ class SystemTagsRelationsCollection extends SimpleCollection {
 				$tagManager,
 				$tagMapper,
 				$userSession,
-				$groupManager
+				$groupManager,
+				$fileRoot
 			),
 		];
 
diff --git a/apps/dav/tests/unit/systemtag/systemtagplugin.php b/apps/dav/tests/unit/systemtag/systemtagplugin.php
index 873dd7088a8..b026451701f 100644
--- a/apps/dav/tests/unit/systemtag/systemtagplugin.php
+++ b/apps/dav/tests/unit/systemtag/systemtagplugin.php
@@ -272,6 +272,40 @@ class SystemTagPlugin extends \Test\TestCase {
 	}
 
 	/**
+	 * @expectedException \Sabre\DAV\Exception\NotFound
+	 */
+	public function testCreateTagToUnknownNode() {
+		$systemTag = new SystemTag(1, 'Test', true, false);
+
+		$node = $this->getMockBuilder('\OCA\DAV\SystemTag\SystemTagsObjectMappingCollection')
+			->disableOriginalConstructor()
+			->getMock();
+
+		$this->tree->expects($this->any())
+			->method('getNodeForPath')
+			->will($this->throwException(new \Sabre\DAV\Exception\NotFound()));
+
+		$this->tagManager->expects($this->never())
+			->method('createTag');
+
+		$node->expects($this->never())
+			->method('createFile');
+
+		$request = $this->getMockBuilder('Sabre\HTTP\RequestInterface')
+				->disableOriginalConstructor()
+				->getMock();
+		$response = $this->getMockBuilder('Sabre\HTTP\ResponseInterface')
+				->disableOriginalConstructor()
+				->getMock();
+
+		$request->expects($this->once())
+			->method('getPath')
+			->will($this->returnValue('/systemtags-relations/files/12'));
+
+		$this->plugin->httpPost($request, $response);
+	}
+
+	/**
 	 * @dataProvider nodeClassProvider
 	 * @expectedException Sabre\DAV\Exception\Conflict
 	 */
diff --git a/apps/dav/tests/unit/systemtag/systemtagsobjecttypecollection.php b/apps/dav/tests/unit/systemtag/systemtagsobjecttypecollection.php
index e6d94803cc0..1d4264f94f9 100644
--- a/apps/dav/tests/unit/systemtag/systemtagsobjecttypecollection.php
+++ b/apps/dav/tests/unit/systemtag/systemtagsobjecttypecollection.php
@@ -38,6 +38,11 @@ class SystemTagsObjectTypeCollection extends \Test\TestCase {
 	 */
 	private $tagMapper;
 
+	/**
+	 * @var \OCP\Files\Folder
+	 */
+	private $userFolder;
+
 	protected function setUp() {
 		parent::setUp();
 
@@ -58,12 +63,21 @@ class SystemTagsObjectTypeCollection extends \Test\TestCase {
 			->with('testuser')
 			->will($this->returnValue(true));
 
+		$this->userFolder = $this->getMock('\OCP\Files\Folder');
+
+		$fileRoot = $this->getMock('\OCP\Files\IRootFolder');
+		$fileRoot->expects($this->any())
+			->method('getUserfolder')
+			->with('testuser')
+			->will($this->returnValue($this->userFolder));
+
 		$this->node = new \OCA\DAV\SystemTag\SystemTagsObjectTypeCollection(
 			'files',
 			$this->tagManager,
 			$this->tagMapper,
 			$userSession,
-			$groupManager
+			$groupManager,
+			$fileRoot
 		);
 	}
 
@@ -82,10 +96,25 @@ class SystemTagsObjectTypeCollection extends \Test\TestCase {
 	}
 
 	public function testGetChild() {
-		$childNode = $this->node->getChild('files');
+		$this->userFolder->expects($this->once())
+			->method('getById')
+			->with('555')
+			->will($this->returnValue([true]));
+		$childNode = $this->node->getChild('555');
 
 		$this->assertInstanceOf('\OCA\DAV\SystemTag\SystemTagsObjectMappingCollection', $childNode);
-		$this->assertEquals('files', $childNode->getName());
+		$this->assertEquals('555', $childNode->getName());
+	}
+
+	/**
+	 * @expectedException Sabre\DAV\Exception\NotFound
+	 */
+	public function testGetChildWithoutAccess() {
+		$this->userFolder->expects($this->once())
+			->method('getById')
+			->with('555')
+			->will($this->returnValue([]));
+		$this->node->getChild('555');
 	}
 
 	/**
@@ -96,9 +125,21 @@ class SystemTagsObjectTypeCollection extends \Test\TestCase {
 	}
 
 	public function testChildExists() {
+		$this->userFolder->expects($this->once())
+			->method('getById')
+			->with('123')
+			->will($this->returnValue([true]));
 		$this->assertTrue($this->node->childExists('123'));
 	}
 
+	public function testChildExistsWithoutAccess() {
+		$this->userFolder->expects($this->once())
+			->method('getById')
+			->with('555')
+			->will($this->returnValue([]));
+		$this->assertFalse($this->node->childExists('555'));
+	}
+
 	/**
 	 * @expectedException Sabre\DAV\Exception\Forbidden
 	 */