Make sure user has access to file for system tag operations

Fixes DAV's SystemTagsObjectTypeCollection to not give access to files
where the current user doesn't have access to.

diff --git a/apps/dav/lib/rootcollection.php b/apps/dav/lib/rootcollection.php
index b7c2995dbad667365e5a1fec382252f67d64538d..5be469e7b0c6aaa399b4005d57c004c436b7c8ed 100644 (file)
--- 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 4636ed428b1cda606d06414ebb11ebceb35a6c21..3348b431c47145ec00dcb973419c727edf1e3ceb 100644 (file)
--- 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 166e3219bc1ef18a12bcf4309ce9eb2cdee8022d..93cc75619289187f60204e42ae4f7a3e5aa316a2 100644 (file)
--- 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
@@ -62,6 +64,11 @@ class SystemTagsObjectTypeCollection implements ICollection {
         */
        private $userSession;
 
+       /**
+        * @var IRootFolder
+        **/
+       protected $fileRoot;
+
        /**
         * Constructor
         *
@@ -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 223aaac93d600fb42044aa3d8ba381b141a0e569..6af4edfc1ab29bea03be3b74b45a38d41d218bb9 100644 (file)
--- 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 873dd7088a87a78de6a8336d8e46b6d37971f66b..b026451701f514973e7525e229c3ec9fd91631d4 100644 (file)
--- a/apps/dav/tests/unit/systemtag/systemtagplugin.php
+++ b/apps/dav/tests/unit/systemtag/systemtagplugin.php
@@ -271,6 +271,40 @@ class SystemTagPlugin extends \Test\TestCase {
                $this->plugin->httpPost($request, $response);
        }
 
+       /**
+        * @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 e6d94803cc0100cfcb7655bbf6a95222cc263cbb..1d4264f94f92b2b1f5f89f97fee31623cb56c74f 100644 (file)
--- 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
         */