Skip to content

Commit

Permalink
Merge pull request #33994 from owncloud/secure_view_plugin
Browse files Browse the repository at this point in the history
View-only dav plugin using IShare attributes
  • Loading branch information
Vincent Petry authored Apr 2, 2019
2 parents c6bdf34 + 33795df commit b22bf45
Show file tree
Hide file tree
Showing 29 changed files with 1,538 additions and 84 deletions.
6 changes: 6 additions & 0 deletions apps/dav/lib/Connector/Sabre/ServerFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

use OCA\DAV\DAV\FileCustomPropertiesBackend;
use OCA\DAV\DAV\FileCustomPropertiesPlugin;
use OCA\DAV\DAV\ViewOnlyPlugin;
use OCA\DAV\Files\BrowserErrorPagePlugin;
use OCA\DAV\Files\FileLocksBackend;
use OCP\Files\Mount\IMountManager;
Expand Down Expand Up @@ -162,6 +163,11 @@ public function createServer($baseUri,
);
$server->addPlugin(new \OCA\DAV\Connector\Sabre\QuotaPlugin($view));

// Allow view-only plugin for webdav requests
$server->addPlugin(new ViewOnlyPlugin(
\OC::$server->getLogger()
));

if ($this->userSession->isLoggedIn()) {
$server->addPlugin(new \OCA\DAV\Connector\Sabre\TagsPlugin($objectTree, $this->tagManager));
$server->addPlugin(new \OCA\DAV\Connector\Sabre\SharesPlugin(
Expand Down
116 changes: 116 additions & 0 deletions apps/dav/lib/DAV/ViewOnlyPlugin.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
<?php
/**
* @author Piotr Mrowczynski piotr@owncloud.com
*
* @copyright Copyright (c) 2019, ownCloud GmbH
* @license AGPL-3.0
*
* This code is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License, version 3,
* as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License, version 3,
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/

namespace OCA\DAV\DAV;

use OCA\DAV\Connector\Sabre\Exception\Forbidden;
use OCA\DAV\Connector\Sabre\File as DavFile;
use OCA\DAV\Meta\MetaFile;
use OCP\Files\FileInfo;
use OCP\Files\NotFoundException;
use OCP\ILogger;
use Sabre\DAV\Server;
use Sabre\DAV\ServerPlugin;
use Sabre\HTTP\RequestInterface;
use Sabre\DAV\Exception\NotFound;

/**
* Sabre plugin for restricting file share receiver download:
*/
class ViewOnlyPlugin extends ServerPlugin {

/** @var Server $server */
private $server;

/** @var ILogger $logger */
private $logger;

/**
* @param ILogger $logger
*/
public function __construct(ILogger $logger) {
$this->logger = $logger;
}

/**
* This initializes the plugin.
*
* This function is called by Sabre\DAV\Server, after
* addPlugin is called.
*
* This method should set up the required event subscriptions.
*
* @param Server $server
* @return void
*/
public function initialize(Server $server) {
$this->server = $server;
//priority 90 to make sure the plugin is called before
//Sabre\DAV\CorePlugin::httpGet
$this->server->on('method:GET', [$this, 'checkViewOnly'], 90);
}

/**
* Disallow download via DAV Api in case file being received share
* and having special permission
*
* @param RequestInterface $request request object
* @return boolean
* @throws Forbidden
* @throws NotFoundException
*/
public function checkViewOnly(
RequestInterface $request
) {
$path = $request->getPath();

try {
$davNode = $this->server->tree->getNodeForPath($path);
if (!($davNode instanceof DavFile || $davNode instanceof MetaFile)) {
return true;
}
// Restrict view-only to nodes which are shared
$node = $davNode->getNode();
if (!$node instanceof FileInfo) {
return true;
}

$storage = $node->getStorage();
// using string as we have no guarantee that "files_sharing" app is loaded
if (!$storage->instanceOfStorage('OCA\Files_Sharing\SharedStorage')) {
return true;
}
// Extract extra permissions
/** @var \OCA\Files_Sharing\SharedStorage $storage */
$share = $storage->getShare();

// Check if read-only and on whether permission can download is both set and disabled.
$canDownload = $share->getAttributes()->getAttribute('permissions', 'download');
if ($canDownload !== null && !$canDownload) {
throw new Forbidden('Access to this resource has been denied because it is in view-only mode.');
}
} catch (NotFound $e) {
$this->logger->warning($e->getMessage());
}

return true;
}
}
6 changes: 6 additions & 0 deletions apps/dav/lib/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
use OCA\DAV\DAV\LazyOpsPlugin;
use OCA\DAV\DAV\MiscCustomPropertiesBackend;
use OCA\DAV\DAV\PublicAuth;
use OCA\DAV\DAV\ViewOnlyPlugin;
use OCA\DAV\Files\BrowserErrorPagePlugin;
use OCA\DAV\Files\FileLocksBackend;
use OCA\DAV\Files\PreviewPlugin;
Expand Down Expand Up @@ -189,6 +190,11 @@ public function __construct(IRequest $request, $baseUri) {
$this->server->addPlugin(new CopyEtagHeaderPlugin());
$this->server->addPlugin(new ChunkingPlugin());

// Allow view-only plugin for webdav requests
$this->server->addPlugin(new ViewOnlyPlugin(
\OC::$server->getLogger()
));

if (BrowserErrorPagePlugin::isBrowserRequest($request)) {
$this->server->addPlugin(new BrowserErrorPagePlugin());
}
Expand Down
128 changes: 128 additions & 0 deletions apps/dav/tests/unit/DAV/ViewOnlyPluginTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
<?php
/**
* @author Piotr Mrowczynski piotr@owncloud.com
*
* @copyright Copyright (c) 2019, ownCloud GmbH
* @license AGPL-3.0
*
* This code is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License, version 3,
* as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License, version 3,
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/
namespace OCA\DAV\Tests\unit\DAV;

use OCA\DAV\DAV\ViewOnlyPlugin;
use OCA\Files_Sharing\SharedStorage;
use OCA\DAV\Connector\Sabre\File as DavFile;
use OCP\Files\FileInfo;
use OCP\Files\Storage\IStorage;
use OCP\ILogger;
use OCP\Share\IAttributes;
use OCP\Share\IShare;
use Sabre\DAV\Server;
use Sabre\DAV\Tree;
use Test\TestCase;
use Sabre\HTTP\RequestInterface;
use OCA\DAV\Connector\Sabre\Exception\Forbidden;

class ViewOnlyPluginTest extends TestCase {

/** @var ViewOnlyPlugin */
private $plugin;
/** @var Tree | \PHPUnit\Framework\MockObject\MockObject */
private $tree;
/** @var RequestInterface | \PHPUnit\Framework\MockObject\MockObject */
private $request;

public function setUp() {
$this->plugin = new ViewOnlyPlugin(
$this->createMock(ILogger::class)
);
$this->request = $this->createMock(RequestInterface::class);
$this->tree = $this->createMock(Tree::class);

$server = $this->createMock(Server::class);
$server->tree = $this->tree;

$this->plugin->initialize($server);
}

public function testCanGetNonDav() {
$this->request->expects($this->once())->method('getPath')->willReturn('files/test/target');
$this->tree->method('getNodeForPath')->willReturn(null);

$this->assertTrue($this->plugin->checkViewOnly($this->request));
}

public function testCanGetNonFileInfo() {
$this->request->expects($this->once())->method('getPath')->willReturn('files/test/target');
$davNode = $this->createMock(DavFile::class);
$this->tree->method('getNodeForPath')->willReturn($davNode);

$davNode->method('getNode')->willReturn(null);

$this->assertTrue($this->plugin->checkViewOnly($this->request));
}

public function testCanGetNonShared() {
$this->request->expects($this->once())->method('getPath')->willReturn('files/test/target');
$davNode = $this->createMock(DavFile::class);
$this->tree->method('getNodeForPath')->willReturn($davNode);

$fileInfo = $this->createMock(FileInfo::class);
$davNode->method('getNode')->willReturn($fileInfo);

$storage = $this->createMock(IStorage::class);
$fileInfo->method('getStorage')->willReturn($storage);
$storage->method('instanceOfStorage')->with(SharedStorage::class)->willReturn(false);

$this->assertTrue($this->plugin->checkViewOnly($this->request));
}

public function providesDataForCanGet() {
return [
// has attribute permissions-download enabled - can get file
[ $this->createMock(FileInfo::class), true, true],
// has no attribute permissions-download - can get file
[ $this->createMock(FileInfo::class), null, true],
// has attribute permissions-download disabled- cannot get the file
[ $this->createMock(FileInfo::class), false, false],
];
}

/**
* @dataProvider providesDataForCanGet
*/
public function testCanGet($nodeInfo, $attrEnabled, $expectCanDownloadFile) {
$this->request->expects($this->once())->method('getPath')->willReturn('files/test/target');

$davNode = $this->createMock(DavFile::class);
$this->tree->method('getNodeForPath')->willReturn($davNode);

$davNode->method('getNode')->willReturn($nodeInfo);

$storage = $this->createMock(SharedStorage::class);
$share = $this->createMock(IShare::class);
$nodeInfo->method('getStorage')->willReturn($storage);
$storage->method('instanceOfStorage')->with(SharedStorage::class)->willReturn(true);
$storage->method('getShare')->willReturn($share);

$extAttr = $this->createMock(IAttributes::class);
$share->method('getAttributes')->willReturn($extAttr);
$extAttr->method('getAttribute')->with('permissions', 'download')->willReturn($attrEnabled);

if (!$expectCanDownloadFile) {
$this->expectException(Forbidden::class);
}
$this->plugin->checkViewOnly($this->request);
}
}
11 changes: 11 additions & 0 deletions apps/files/download.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,17 @@
exit;
}

//Dispatch an event to see if any apps have problem with download
$event = new \Symfony\Component\EventDispatcher\GenericEvent(null, ['path' => $filename]);
OC::$server->getEventDispatcher()->dispatch('file.beforeGetDirect', $event);
if ($event->hasArgument('errorMessage')) {
\header("HTTP/1.0 403 Forbidden");
$tmpl = new OCP\Template('', '403', 'guest');
$tmpl->assign('file', $filename);
$tmpl->printPage();
exit;
}

$ftype=\OC::$server->getMimeTypeDetector()->getSecureMimeType(\OC\Files\Filesystem::getMimeType($filename));

\header('Content-Type:'.$ftype);
Expand Down
9 changes: 9 additions & 0 deletions apps/files/js/fileinfomodel.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,15 @@
return OC.joinPaths(this.get('path'), this.get('name'));
},

/**
* Returns the mimetype of the file
*
* @return {string} mimetype
*/
getMimeType: function() {
return this.get('mimetype');
},

/**
* Reloads missing properties from server and set them in the model.
* @param properties array of properties to be reloaded
Expand Down
3 changes: 2 additions & 1 deletion apps/files_sharing/lib/AppInfo/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,8 @@ function () use ($c) {
$c->getServer()->getUrlGenerator(),
$c->getServer()->getEventDispatcher(),
$c->getServer()->getShareManager(),
$c->query(NotificationPublisher::class)
$c->query(NotificationPublisher::class),
$c->getServer()->getUserSession()
);
});

Expand Down
30 changes: 30 additions & 0 deletions apps/files_sharing/lib/Controller/Share20OcsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,11 @@ protected function formatShare(IShare $share, $received = false) {

$result['mail_send'] = $share->getMailSend() ? 1 : 0;

$result['attributes'] = null;
if ($attributes = $share->getAttributes()) {
$result['attributes'] = \json_encode($attributes->toArray());
}

return $result;
}

Expand Down Expand Up @@ -499,6 +504,8 @@ public function createShare() {
$share->setShareType($shareType);
$share->setSharedBy($this->userSession->getUser()->getUID());

$share = $this->setShareAttributes($share, $this->request->getParam('attributes', null));

try {
$share = $this->shareManager->createShare($share);
/**
Expand Down Expand Up @@ -869,6 +876,8 @@ public function updateShare($id) {
return new Result(null, 400, $this->l->t('Cannot remove all permissions'));
}

$share = $this->setShareAttributes($share, $this->request->getParam('attributes', null));

try {
$share = $this->shareManager->updateShare($share);
} catch (\Exception $e) {
Expand Down Expand Up @@ -1199,4 +1208,25 @@ private function getShareById($id, $recipient = null) {

return $share;
}

/**
* @param IShare $share
* @param string[][]|null $formattedShareAttributes
* @return IShare modified share
*/
private function setShareAttributes(IShare $share, $formattedShareAttributes) {
$newShareAttributes = $this->shareManager->newShare()->newAttributes();
if ($formattedShareAttributes !== null) {
foreach ($formattedShareAttributes as $formattedAttr) {
$newShareAttributes->setAttribute(
$formattedAttr['scope'],
$formattedAttr['key'],
(bool) \json_decode($formattedAttr['enabled'])
);
}
}
$share->setAttributes($newShareAttributes);

return $share;
}
}
Loading

0 comments on commit b22bf45

Please sign in to comment.