Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add share attributes + prevent download permission #32482

Merged
merged 13 commits into from
Aug 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/dav/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@
'OCA\\DAV\\DAV\\Sharing\\Xml\\Invite' => $baseDir . '/../lib/DAV/Sharing/Xml/Invite.php',
'OCA\\DAV\\DAV\\Sharing\\Xml\\ShareRequest' => $baseDir . '/../lib/DAV/Sharing/Xml/ShareRequest.php',
'OCA\\DAV\\DAV\\SystemPrincipalBackend' => $baseDir . '/../lib/DAV/SystemPrincipalBackend.php',
'OCA\\DAV\\DAV\\ViewOnlyPlugin' => $baseDir . '/../lib/DAV/ViewOnlyPlugin.php',
'OCA\\DAV\\Db\\Direct' => $baseDir . '/../lib/Db/Direct.php',
'OCA\\DAV\\Db\\DirectMapper' => $baseDir . '/../lib/Db/DirectMapper.php',
'OCA\\DAV\\Direct\\DirectFile' => $baseDir . '/../lib/Direct/DirectFile.php',
Expand Down
1 change: 1 addition & 0 deletions apps/dav/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ class ComposerStaticInitDAV
'OCA\\DAV\\DAV\\Sharing\\Xml\\Invite' => __DIR__ . '/..' . '/../lib/DAV/Sharing/Xml/Invite.php',
'OCA\\DAV\\DAV\\Sharing\\Xml\\ShareRequest' => __DIR__ . '/..' . '/../lib/DAV/Sharing/Xml/ShareRequest.php',
'OCA\\DAV\\DAV\\SystemPrincipalBackend' => __DIR__ . '/..' . '/../lib/DAV/SystemPrincipalBackend.php',
'OCA\\DAV\\DAV\\ViewOnlyPlugin' => __DIR__ . '/..' . '/../lib/DAV/ViewOnlyPlugin.php',
'OCA\\DAV\\Db\\Direct' => __DIR__ . '/..' . '/../lib/Db/Direct.php',
'OCA\\DAV\\Db\\DirectMapper' => __DIR__ . '/..' . '/../lib/Db/DirectMapper.php',
'OCA\\DAV\\Direct\\DirectFile' => __DIR__ . '/..' . '/../lib/Direct/DirectFile.php',
Expand Down
6 changes: 6 additions & 0 deletions apps/dav/lib/Connector/Sabre/FilesPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ class FilesPlugin extends ServerPlugin {
public const PERMISSIONS_PROPERTYNAME = '{http://owncloud.org/ns}permissions';
public const SHARE_PERMISSIONS_PROPERTYNAME = '{http://open-collaboration-services.org/ns}share-permissions';
public const OCM_SHARE_PERMISSIONS_PROPERTYNAME = '{http://open-cloud-mesh.org/ns}share-permissions';
public const SHARE_ATTRIBUTES_PROPERTYNAME = '{http://nextcloud.org/ns}share-attributes';
public const DOWNLOADURL_PROPERTYNAME = '{http://owncloud.org/ns}downloadURL';
public const SIZE_PROPERTYNAME = '{http://owncloud.org/ns}size';
public const GETETAG_PROPERTYNAME = '{DAV:}getetag';
Expand Down Expand Up @@ -134,6 +135,7 @@ public function initialize(Server $server) {
$server->protectedProperties[] = self::PERMISSIONS_PROPERTYNAME;
$server->protectedProperties[] = self::SHARE_PERMISSIONS_PROPERTYNAME;
$server->protectedProperties[] = self::OCM_SHARE_PERMISSIONS_PROPERTYNAME;
$server->protectedProperties[] = self::SHARE_ATTRIBUTES_PROPERTYNAME;
$server->protectedProperties[] = self::SIZE_PROPERTYNAME;
$server->protectedProperties[] = self::DOWNLOADURL_PROPERTYNAME;
$server->protectedProperties[] = self::OWNER_ID_PROPERTYNAME;
Expand Down Expand Up @@ -321,6 +323,10 @@ public function handleGetProperties(PropFind $propFind, \Sabre\DAV\INode $node)
return json_encode($ocmPermissions);
});

$propFind->handle(self::SHARE_ATTRIBUTES_PROPERTYNAME, function () use ($node, $httpRequest) {
return json_encode($node->getShareAttributes());
});

$propFind->handle(self::GETETAG_PROPERTYNAME, function () use ($node): string {
return $node->getETag();
});
Expand Down
26 changes: 26 additions & 0 deletions apps/dav/lib/Connector/Sabre/Node.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
use OC\Files\Mount\MoveableMount;
use OC\Files\Node\File;
use OC\Files\Node\Folder;
use OC\Files\Storage\Wrapper\Wrapper;
use OC\Files\View;
use OCA\DAV\Connector\Sabre\Exception\InvalidPath;
use OCP\Files\FileInfo;
Expand Down Expand Up @@ -322,6 +323,31 @@ public function getSharePermissions($user) {
return $permissions;
}

/**
* @return array
*/
public function getShareAttributes(): array {
$attributes = [];

try {
$storage = $this->info->getStorage();
} catch (StorageNotAvailableException $e) {
$storage = null;
}

if ($storage && $storage->instanceOfStorage(\OCA\Files_Sharing\SharedStorage::class)) {
/** @var \OCA\Files_Sharing\SharedStorage $storage */
$attributes = $storage->getShare()->getAttributes();
if ($attributes === null) {
return [];
} else {
return $attributes->toArray();
}
}

return $attributes;
}

/**
* @param string $user
* @return string
Expand Down
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 @@ -33,6 +33,7 @@

use OCP\Files\Folder;
use OCA\DAV\AppInfo\PluginManager;
use OCA\DAV\DAV\ViewOnlyPlugin;
use OCA\DAV\Files\BrowserErrorPagePlugin;
use OCP\Files\Mount\IMountManager;
use OCP\IConfig;
Expand Down Expand Up @@ -158,6 +159,11 @@ public function createServer(string $baseUri,
$server->addPlugin(new \OCA\DAV\Connector\Sabre\QuotaPlugin($view, true));
$server->addPlugin(new \OCA\DAV\Connector\Sabre\ChecksumUpdatePlugin());

// Allow view-only plugin for webdav requests
$server->addPlugin(new ViewOnlyPlugin(
$this->logger
));

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
17 changes: 16 additions & 1 deletion apps/dav/lib/Controller/DirectController.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,12 @@
use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\OCS\OCSBadRequestException;
use OCP\AppFramework\OCS\OCSNotFoundException;
use OCP\AppFramework\OCS\OCSForbiddenException;
use OCP\AppFramework\OCSController;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\EventDispatcher\GenericEvent;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\Files\Events\BeforeDirectFileDownloadEvent;
use OCP\Files\File;
use OCP\Files\IRootFolder;
use OCP\IRequest;
Expand All @@ -59,6 +63,8 @@ class DirectController extends OCSController {
/** @var IURLGenerator */
private $urlGenerator;

/** @var IEventDispatcher */
private $eventDispatcher;

public function __construct(string $appName,
IRequest $request,
Expand All @@ -67,7 +73,8 @@ public function __construct(string $appName,
DirectMapper $mapper,
ISecureRandom $random,
ITimeFactory $timeFactory,
IURLGenerator $urlGenerator) {
IURLGenerator $urlGenerator,
IEventDispatcher $eventDispatcher) {
parent::__construct($appName, $request);

$this->rootFolder = $rootFolder;
Expand All @@ -76,6 +83,7 @@ public function __construct(string $appName,
$this->random = $random;
$this->timeFactory = $timeFactory;
$this->urlGenerator = $urlGenerator;
$this->eventDispatcher = $eventDispatcher;
}

/**
Expand All @@ -99,6 +107,13 @@ public function getUrl(int $fileId, int $expirationTime = 60 * 60 * 8): DataResp
throw new OCSBadRequestException('Direct download only works for files');
}

$event = new BeforeDirectFileDownloadEvent($userFolder->getRelativePath($file->getPath()));

Check notice

Code scanning / Psalm

PossiblyNullArgument

Argument 1 of OCP\Files\Events\BeforeDirectFileDownloadEvent::__construct cannot be null, possibly null value provided
$this->eventDispatcher->dispatchTyped($event);

if ($event->isSuccessful() === false) {
throw new OCSForbiddenException('Permission denied to download file');
}

//TODO: at some point we should use the directdownlaod function of storages
$direct = new Direct();
$direct->setUserId($this->userId);
Expand Down
108 changes: 108 additions & 0 deletions apps/dav/lib/DAV/ViewOnlyPlugin.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
<?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 Psr\Log\LoggerInterface;
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 {

private ?Server $server = null;
private LoggerInterface $logger;

public function __construct(LoggerInterface $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.
*/
public function initialize(Server $server): void {
$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
*
* @throws Forbidden
* @throws NotFoundException
*/
public function checkViewOnly(RequestInterface $request): bool {
$path = $request->getPath();

try {
assert($this->server !== null);
$davNode = $this->server->tree->getNodeForPath($path);
Fixed Show fixed Hide fixed
Fixed Show fixed Hide fixed
if (!($davNode instanceof DavFile)) {
return true;
}
// Restrict view-only to nodes which are shared
$node = $davNode->getNode();

$storage = $node->getStorage();

if (!$storage->instanceOfStorage(\OCA\Files_Sharing\SharedStorage::class)) {
return true;
}
// Extract extra permissions
/** @var \OCA\Files_Sharing\SharedStorage $storage */
$share = $storage->getShare();

$attributes = $share->getAttributes();
if ($attributes === null) {
return true;
}

// Check if read-only and on whether permission can download is both set and disabled.
$canDownload = $attributes->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(), [
'exception' => $e,
]);
}

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 @@ -62,6 +62,7 @@
use OCA\DAV\Connector\Sabre\TagsPlugin;
use OCA\DAV\DAV\CustomPropertiesBackend;
use OCA\DAV\DAV\PublicAuth;
use OCA\DAV\DAV\ViewOnlyPlugin;
use OCA\DAV\Events\SabrePluginAuthInitEvent;
use OCA\DAV\Files\BrowserErrorPagePlugin;
use OCA\DAV\Files\LazySearchBackend;
Expand Down Expand Up @@ -229,6 +230,11 @@ public function __construct(IRequest $request, string $baseUri) {
$this->server->addPlugin(new FakeLockerPlugin());
}

// Allow view-only plugin for webdav requests
$this->server->addPlugin(new ViewOnlyPlugin(
$logger
));

if (BrowserErrorPagePlugin::isBrowserRequest($request)) {
$this->server->addPlugin(new BrowserErrorPagePlugin());
}
Expand Down
62 changes: 62 additions & 0 deletions apps/dav/tests/unit/Connector/Sabre/NodeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,11 @@

use OC\Files\FileInfo;
use OC\Files\View;
use OC\Share20\ShareAttributes;
use OCA\Files_Sharing\SharedStorage;
use OCP\Files\Mount\IMountPoint;
use OCP\Files\Storage;
use OCP\Share\IAttributes;
use OCP\Share\IManager;
use OCP\Share\IShare;

Expand Down Expand Up @@ -169,6 +172,65 @@ public function testSharePermissions($type, $user, $permissions, $expected) {
$this->assertEquals($expected, $node->getSharePermissions($user));
}

public function testShareAttributes() {
$storage = $this->getMockBuilder(SharedStorage::class)
->disableOriginalConstructor()
->setMethods(['getShare'])
->getMock();

$shareManager = $this->getMockBuilder(IManager::class)->disableOriginalConstructor()->getMock();
$share = $this->getMockBuilder(IShare::class)->disableOriginalConstructor()->getMock();

$storage->expects($this->once())
->method('getShare')
->willReturn($share);

$attributes = new ShareAttributes();
$attributes->setAttribute('permissions', 'download', false);

$share->expects($this->once())->method('getAttributes')->willReturn($attributes);

$info = $this->getMockBuilder(FileInfo::class)
->disableOriginalConstructor()
->setMethods(['getStorage', 'getType'])
->getMock();

$info->method('getStorage')->willReturn($storage);
$info->method('getType')->willReturn(FileInfo::TYPE_FOLDER);

$view = $this->getMockBuilder(View::class)
->disableOriginalConstructor()
->getMock();

$node = new \OCA\DAV\Connector\Sabre\File($view, $info);
$this->invokePrivate($node, 'shareManager', [$shareManager]);
$this->assertEquals($attributes->toArray(), $node->getShareAttributes());
}

public function testShareAttributesNonShare() {
$storage = $this->getMockBuilder(Storage::class)
->disableOriginalConstructor()
->getMock();

$shareManager = $this->getMockBuilder(IManager::class)->disableOriginalConstructor()->getMock();

$info = $this->getMockBuilder(FileInfo::class)
->disableOriginalConstructor()
->setMethods(['getStorage', 'getType'])
->getMock();

$info->method('getStorage')->willReturn($storage);
$info->method('getType')->willReturn(FileInfo::TYPE_FOLDER);

$view = $this->getMockBuilder(View::class)
->disableOriginalConstructor()
->getMock();

$node = new \OCA\DAV\Connector\Sabre\File($view, $info);
$this->invokePrivate($node, 'shareManager', [$shareManager]);
$this->assertEquals([], $node->getShareAttributes());
}

public function sanitizeMtimeProvider() {
return [
[123456789, 123456789],
Expand Down
Loading