Properly handle missing READ permission

This commit is contained in:
Vincent Petry 2017-02-24 11:56:29 +01:00 committed by Joas Schilling
parent 53deb26778
commit 614bd5c294
No known key found for this signature in database
GPG Key ID: E166FD8976B3BAC8
7 changed files with 100 additions and 11 deletions

View File

@ -44,6 +44,7 @@ use Sabre\DAV\INode;
use Sabre\DAV\Exception\BadRequest;
use OC\Files\Mount\MoveableMount;
use Sabre\DAV\IFile;
use Sabre\DAV\Exception\NotFound;
class Directory extends \OCA\DAV\Connector\Sabre\Node
implements \Sabre\DAV\ICollection, \Sabre\DAV\IQuota, \Sabre\DAV\IMoveTarget {
@ -199,6 +200,11 @@ class Directory extends \OCA\DAV\Connector\Sabre\Node
* @throws \Sabre\DAV\Exception\ServiceUnavailable
*/
public function getChild($name, $info = null) {
if (!$this->info->isReadable()) {
// avoid detecting files through this way
throw new NotFound();
}
$path = $this->path . '/' . $name;
if (is_null($info)) {
try {
@ -232,12 +238,17 @@ class Directory extends \OCA\DAV\Connector\Sabre\Node
* Returns an array with all the child nodes
*
* @return \Sabre\DAV\INode[]
* @throws \Sabre\DAV\Exception\Locked
* @throws \OCA\DAV\Connector\Sabre\Exception\Forbidden
*/
public function getChildren() {
if (!is_null($this->dirContent)) {
return $this->dirContent;
}
try {
if (!$this->info->isReadable()) {
throw new Forbidden('No read permissions');
}
$folderContent = $this->fileView->getDirectoryContent($this->path);
} catch (LockedException $e) {
throw new Locked();

View File

@ -54,6 +54,7 @@ use Sabre\DAV\Exception\Forbidden;
use Sabre\DAV\Exception\NotImplemented;
use Sabre\DAV\Exception\ServiceUnavailable;
use Sabre\DAV\IFile;
use Sabre\DAV\Exception\NotFound;
class File extends Node implements IFile {
@ -307,6 +308,10 @@ class File extends Node implements IFile {
public function get() {
//throw exception if encryption is disabled but files are still encrypted
try {
if (!$this->info->isReadable()) {
// do a if the file did not exist
throw new NotFound();
}
$res = $this->fileView->fopen(ltrim($this->path, '/'), 'rb');
if ($res === false) {
throw new ServiceUnavailable("Could not open file");

View File

@ -286,6 +286,10 @@ class FilesPlugin extends ServerPlugin {
$httpRequest = $this->server->httpRequest;
if ($node instanceof \OCA\DAV\Connector\Sabre\Node) {
if (!$node->getFileInfo()->isReadable()) {
// avoid detecting files through this means
throw new NotFound();
}
$propFind->handle(self::FILEID_PROPERTYNAME, function() use ($node) {
return $node->getFileId();

View File

@ -77,12 +77,11 @@ class DirectoryTest extends \Test\TestCase {
protected function setUp() {
parent::setUp();
$this->view = $this->getMockBuilder('OC\Files\View')
->disableOriginalConstructor()
->getMock();
$this->info = $this->getMockBuilder('OC\Files\FileInfo')
->disableOriginalConstructor()
->getMock();
$this->view = $this->createMock('OC\Files\View');
$this->info = $this->createMock('OC\Files\FileInfo');
$this->info->expects($this->any())
->method('isReadable')
->will($this->returnValue(true));
}
private function getDir($path = '/') {

View File

@ -1003,4 +1003,23 @@ class FileTest extends \Test\TestCase {
$file->get();
}
/**
* @expectedException \Sabre\DAV\Exception\NotFound
*/
public function testGetThrowsIfNoPermission() {
$view = $this->getMockBuilder(View::class)
->setMethods(['fopen'])
->getMock();
$view->expects($this->never())
->method('fopen');
$info = new FileInfo('/test.txt', $this->getMockStorage(), null, [
'permissions' => Constants::PERMISSION_CREATE // no read perm
], null);
$file = new File($view, $info);
$file->get();
}
}

View File

@ -34,6 +34,7 @@ use Sabre\HTTP\ResponseInterface;
use Test\TestCase;
use OCA\DAV\Upload\FutureFile;
use OCA\DAV\Connector\Sabre\Directory;
use OCP\Files\FileInfo;
/**
* Copyright (c) 2015 Vincent Petry <pvince81@owncloud.com>
@ -148,13 +149,15 @@ class FilesPluginTest extends TestCase {
$node->expects($this->any())
->method('getDavPermissions')
->will($this->returnValue('DWCKMSR'));
$fileInfo = $this->createMock(FileInfo::class);
$fileInfo->expects($this->any())
->method('isReadable')
->willReturn(true);
$node->expects($this->any())
->method('getFileInfo')
->will($this->returnValue(
$this->getMockBuilder('\OCP\Files\FileInfo')
->disableOriginalConstructor()
->getMock()
));
->willReturn($fileInfo);
return $node;
}
@ -313,6 +316,15 @@ class FilesPluginTest extends TestCase {
->getMock();
$node->expects($this->any())->method('getPath')->willReturn('/');
$fileInfo = $this->createMock(FileInfo::class);
$fileInfo->expects($this->any())
->method('isReadable')
->willReturn(true);
$node->expects($this->any())
->method('getFileInfo')
->willReturn($fileInfo);
$propFind = new PropFind(
'/',
[
@ -329,6 +341,39 @@ class FilesPluginTest extends TestCase {
$this->assertEquals('my_fingerprint', $propFind->get(self::DATA_FINGERPRINT_PROPERTYNAME));
}
/**
* @expectedException \Sabre\DAV\Exception\NotFound
*/
public function testGetPropertiesWhenNoPermission() {
/** @var \OCA\DAV\Connector\Sabre\Directory | \PHPUnit_Framework_MockObject_MockObject $node */
$node = $this->getMockBuilder('\OCA\DAV\Connector\Sabre\Directory')
->disableOriginalConstructor()
->getMock();
$node->expects($this->any())->method('getPath')->willReturn('/');
$fileInfo = $this->createMock(FileInfo::class);
$fileInfo->expects($this->any())
->method('isReadable')
->willReturn(false);
$node->expects($this->any())
->method('getFileInfo')
->willReturn($fileInfo);
$propFind = new PropFind(
'/test',
[
self::DATA_FINGERPRINT_PROPERTYNAME,
],
0
);
$this->plugin->handleGetProperties(
$propFind,
$node
);
}
public function testUpdateProps() {
$node = $this->createTestNode('\OCA\DAV\Connector\Sabre\File');

View File

@ -34,6 +34,7 @@ use OCP\Files\Folder;
use OCP\IGroupManager;
use OCP\SystemTag\ISystemTagManager;
use OCP\ITags;
use OCP\Files\FileInfo;
class FilesReportPluginTest extends \Test\TestCase {
/** @var \Sabre\DAV\Server|\PHPUnit_Framework_MockObject_MockObject */
@ -349,6 +350,9 @@ class FilesReportPluginTest extends \Test\TestCase {
public function testPrepareResponses() {
$requestedProps = ['{DAV:}getcontentlength', '{http://owncloud.org/ns}fileid', '{DAV:}resourcetype'];
$fileInfo = $this->createMock(FileInfo::class);
$fileInfo->method('isReadable')->willReturn(true);
$node1 = $this->getMockBuilder('\OCA\DAV\Connector\Sabre\Directory')
->disableOriginalConstructor()
->getMock();
@ -362,6 +366,7 @@ class FilesReportPluginTest extends \Test\TestCase {
$node1->expects($this->any())
->method('getPath')
->will($this->returnValue('/node1'));
$node1->method('getFileInfo')->willReturn($fileInfo);
$node2->expects($this->once())
->method('getInternalFileId')
->will($this->returnValue('222'));
@ -371,6 +376,7 @@ class FilesReportPluginTest extends \Test\TestCase {
$node2->expects($this->any())
->method('getPath')
->will($this->returnValue('/sub/node2'));
$node2->method('getFileInfo')->willReturn($fileInfo);
$config = $this->getMockBuilder('\OCP\IConfig')
->disableOriginalConstructor()