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.
This commit is contained in:
Vincent Petry 2016-02-01 18:18:17 +01:00
parent b4853f3fce
commit d72c0ffbc6
6 changed files with 109 additions and 13 deletions

View file

@ -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(),

View file

@ -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();

View file

@ -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;
}

View file

@ -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
),
];

View file

@ -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

View file

@ -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
*/