diff options
Diffstat (limited to 'apps')
38 files changed, 1387 insertions, 196 deletions
diff --git a/apps/dav/appinfo/v1/publicwebdav.php b/apps/dav/appinfo/v1/publicwebdav.php index c99c5bcd833..b63a5895e17 100644 --- a/apps/dav/appinfo/v1/publicwebdav.php +++ b/apps/dav/appinfo/v1/publicwebdav.php @@ -29,6 +29,9 @@ * along with this program. If not, see <http://www.gnu.org/licenses/> * */ + +use Psr\Log\LoggerInterface; + // load needed apps $RUNTIME_APPTYPES = ['filesystem', 'authentication', 'logging']; @@ -49,6 +52,7 @@ $authPlugin = new \Sabre\DAV\Auth\Plugin($authBackend); $serverFactory = new OCA\DAV\Connector\Sabre\ServerFactory( \OC::$server->getConfig(), \OC::$server->getLogger(), + \OC::$server->get(LoggerInterface::class), \OC::$server->getDatabaseConnection(), \OC::$server->getUserSession(), \OC::$server->getMountManager(), diff --git a/apps/dav/appinfo/v1/webdav.php b/apps/dav/appinfo/v1/webdav.php index 7b0a0ae11df..15a3aee21bc 100644 --- a/apps/dav/appinfo/v1/webdav.php +++ b/apps/dav/appinfo/v1/webdav.php @@ -28,6 +28,9 @@ * along with this program. If not, see <http://www.gnu.org/licenses/> * */ + +use Psr\Log\LoggerInterface; + // no php execution timeout for webdav if (strpos(@ini_get('disable_functions'), 'set_time_limit') === false) { @set_time_limit(0); @@ -40,6 +43,7 @@ ignore_user_abort(true); $serverFactory = new \OCA\DAV\Connector\Sabre\ServerFactory( \OC::$server->getConfig(), \OC::$server->getLogger(), + \OC::$server->get(LoggerInterface::class), \OC::$server->getDatabaseConnection(), \OC::$server->getUserSession(), \OC::$server->getMountManager(), diff --git a/apps/dav/composer/composer/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php index 1539cdc2a76..ce7617c20dc 100644 --- a/apps/dav/composer/composer/autoload_classmap.php +++ b/apps/dav/composer/composer/autoload_classmap.php @@ -191,6 +191,7 @@ return array( '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', diff --git a/apps/dav/composer/composer/autoload_static.php b/apps/dav/composer/composer/autoload_static.php index d432fc55324..d1e4cc10e27 100644 --- a/apps/dav/composer/composer/autoload_static.php +++ b/apps/dav/composer/composer/autoload_static.php @@ -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', diff --git a/apps/dav/lib/Connector/Sabre/FilesPlugin.php b/apps/dav/lib/Connector/Sabre/FilesPlugin.php index 9c4f912610b..ca4e2c968e8 100644 --- a/apps/dav/lib/Connector/Sabre/FilesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/FilesPlugin.php @@ -64,6 +64,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'; @@ -172,6 +173,7 @@ class FilesPlugin extends ServerPlugin { $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; @@ -359,6 +361,10 @@ class FilesPlugin extends ServerPlugin { return json_encode($ocmPermissions); }); + $propFind->handle(self::SHARE_ATTRIBUTES_PROPERTYNAME, function () use ($node, $httpRequest): string { + return json_encode($node->getShareAttributes()); + }); + $propFind->handle(self::GETETAG_PROPERTYNAME, function () use ($node) { return $node->getETag(); }); diff --git a/apps/dav/lib/Connector/Sabre/Node.php b/apps/dav/lib/Connector/Sabre/Node.php index e4517068f42..87f2fea394f 100644 --- a/apps/dav/lib/Connector/Sabre/Node.php +++ b/apps/dav/lib/Connector/Sabre/Node.php @@ -38,6 +38,7 @@ namespace OCA\DAV\Connector\Sabre; 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; @@ -323,6 +324,31 @@ abstract class Node implements \Sabre\DAV\INode { } /** + * @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 */ diff --git a/apps/dav/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php index b13dbd20ca9..1ca850198cf 100644 --- a/apps/dav/lib/Connector/Sabre/ServerFactory.php +++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php @@ -33,6 +33,7 @@ namespace OCA\DAV\Connector\Sabre; 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; @@ -44,6 +45,7 @@ use OCP\IRequest; use OCP\ITagManager; use OCP\IUserSession; use OCP\SabrePluginEvent; +use Psr\Log\LoggerInterface; use Sabre\DAV\Auth\Plugin; use Symfony\Component\EventDispatcher\EventDispatcherInterface; @@ -52,6 +54,8 @@ class ServerFactory { private $config; /** @var ILogger */ private $logger; + /** @var LoggerInterface */ + private $psrLogger; /** @var IDBConnection */ private $databaseConnection; /** @var IUserSession */ @@ -72,6 +76,7 @@ class ServerFactory { /** * @param IConfig $config * @param ILogger $logger + * @param LoggerInterface $psrLogger * @param IDBConnection $databaseConnection * @param IUserSession $userSession * @param IMountManager $mountManager @@ -82,6 +87,7 @@ class ServerFactory { public function __construct( IConfig $config, ILogger $logger, + LoggerInterface $psrLogger, IDBConnection $databaseConnection, IUserSession $userSession, IMountManager $mountManager, @@ -93,6 +99,7 @@ class ServerFactory { ) { $this->config = $config; $this->logger = $logger; + $this->psrLogger = $psrLogger; $this->databaseConnection = $databaseConnection; $this->userSession = $userSession; $this->mountManager = $mountManager; @@ -182,6 +189,11 @@ class ServerFactory { $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->psrLogger + )); + if ($this->userSession->isLoggedIn()) { $server->addPlugin(new \OCA\DAV\Connector\Sabre\TagsPlugin($objectTree, $this->tagManager)); $server->addPlugin(new \OCA\DAV\Connector\Sabre\SharesPlugin( diff --git a/apps/dav/lib/Controller/DirectController.php b/apps/dav/lib/Controller/DirectController.php index 955400998cf..f9c83488935 100644 --- a/apps/dav/lib/Controller/DirectController.php +++ b/apps/dav/lib/Controller/DirectController.php @@ -31,8 +31,12 @@ use OCA\DAV\Db\DirectMapper; 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; @@ -59,6 +63,8 @@ class DirectController extends OCSController { /** @var IURLGenerator */ private $urlGenerator; + /** @var IEventDispatcher */ + private $eventDispatcher; public function __construct(string $appName, IRequest $request, @@ -67,7 +73,8 @@ class DirectController extends OCSController { DirectMapper $mapper, ISecureRandom $random, ITimeFactory $timeFactory, - IURLGenerator $urlGenerator) { + IURLGenerator $urlGenerator, + IEventDispatcher $eventDispatcher) { parent::__construct($appName, $request); $this->rootFolder = $rootFolder; @@ -76,6 +83,7 @@ class DirectController extends OCSController { $this->random = $random; $this->timeFactory = $timeFactory; $this->urlGenerator = $urlGenerator; + $this->eventDispatcher = $eventDispatcher; } /** @@ -99,6 +107,13 @@ class DirectController extends OCSController { throw new OCSBadRequestException('Direct download only works for files'); } + $event = new BeforeDirectFileDownloadEvent($userFolder->getRelativePath($file->getPath())); + $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); diff --git a/apps/dav/lib/DAV/ViewOnlyPlugin.php b/apps/dav/lib/DAV/ViewOnlyPlugin.php new file mode 100644 index 00000000000..1504969b5b4 --- /dev/null +++ b/apps/dav/lib/DAV/ViewOnlyPlugin.php @@ -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); + 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; + } +} diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index 589e6c2bd6c..5eb06349e9c 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -66,6 +66,7 @@ use OCA\DAV\Connector\Sabre\SharesPlugin; 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; @@ -229,6 +230,11 @@ class Server { $this->server->addPlugin(new FakeLockerPlugin()); } + // Allow view-only plugin for webdav requests + $this->server->addPlugin(new ViewOnlyPlugin( + \OC::$server->get(LoggerInterface::class) + )); + if (BrowserErrorPagePlugin::isBrowserRequest($request)) { $this->server->addPlugin(new BrowserErrorPagePlugin()); } diff --git a/apps/dav/tests/unit/Connector/Sabre/NodeTest.php b/apps/dav/tests/unit/Connector/Sabre/NodeTest.php index 00fd0ebd8aa..3ac5b8f841a 100644 --- a/apps/dav/tests/unit/Connector/Sabre/NodeTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/NodeTest.php @@ -29,8 +29,11 @@ namespace OCA\DAV\Tests\unit\Connector\Sabre; 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; @@ -169,6 +172,65 @@ class NodeTest extends \Test\TestCase { $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], diff --git a/apps/dav/tests/unit/Connector/Sabre/RequestTest/RequestTestCase.php b/apps/dav/tests/unit/Connector/Sabre/RequestTest/RequestTestCase.php index 5adfece42e8..e922cbc2835 100644 --- a/apps/dav/tests/unit/Connector/Sabre/RequestTest/RequestTestCase.php +++ b/apps/dav/tests/unit/Connector/Sabre/RequestTest/RequestTestCase.php @@ -36,6 +36,7 @@ use Sabre\HTTP\Request; use Test\TestCase; use Test\Traits\MountProviderTrait; use Test\Traits\UserTrait; +use Psr\Log\LoggerInterface; abstract class RequestTestCase extends TestCase { use UserTrait; @@ -61,6 +62,7 @@ abstract class RequestTestCase extends TestCase { $this->serverFactory = new ServerFactory( \OC::$server->getConfig(), \OC::$server->getLogger(), + \OC::$server->get(LoggerInterface::class), \OC::$server->getDatabaseConnection(), \OC::$server->getUserSession(), \OC::$server->getMountManager(), diff --git a/apps/dav/tests/unit/Controller/DirectControllerTest.php b/apps/dav/tests/unit/Controller/DirectControllerTest.php index 00771e7f7a6..37cf406e84a 100644 --- a/apps/dav/tests/unit/Controller/DirectControllerTest.php +++ b/apps/dav/tests/unit/Controller/DirectControllerTest.php @@ -34,6 +34,7 @@ use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\OCS\OCSBadRequestException; use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\AppFramework\Utility\ITimeFactory; +use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\File; use OCP\Files\Folder; use OCP\Files\IRootFolder; @@ -56,11 +57,13 @@ class DirectControllerTest extends TestCase { /** @var ITimeFactory|\PHPUnit\Framework\MockObject\MockObject */ private $timeFactory; - /** @var IURLGenerator|\PHPUnit\Framework\MockObject\MockObject */ + /** @var IURlGenerator|\PHPUnit\Framework\MockObject\MockObject */ private $urlGenerator; - /** @var DirectController */ - private $controller; + /** @var IEventDispatcher|\PHPUnit\Framework\MockObject\MockObject */ + private $eventDispatcher; + + private DirectController $controller; protected function setUp(): void { parent::setUp(); @@ -70,6 +73,7 @@ class DirectControllerTest extends TestCase { $this->random = $this->createMock(ISecureRandom::class); $this->timeFactory = $this->createMock(ITimeFactory::class); $this->urlGenerator = $this->createMock(IURLGenerator::class); + $this->eventDispatcher = $this->createMock(IEventDispatcher::class); $this->controller = new DirectController( 'dav', @@ -79,11 +83,12 @@ class DirectControllerTest extends TestCase { $this->directMapper, $this->random, $this->timeFactory, - $this->urlGenerator + $this->urlGenerator, + $this->eventDispatcher ); } - public function testGetUrlNonExistingFileId() { + public function testGetUrlNonExistingFileId(): void { $userFolder = $this->createMock(Folder::class); $this->rootFolder->method('getUserFolder') ->with('awesomeUser') @@ -97,7 +102,7 @@ class DirectControllerTest extends TestCase { $this->controller->getUrl(101); } - public function testGetUrlForFolder() { + public function testGetUrlForFolder(): void { $userFolder = $this->createMock(Folder::class); $this->rootFolder->method('getUserFolder') ->with('awesomeUser') @@ -113,7 +118,7 @@ class DirectControllerTest extends TestCase { $this->controller->getUrl(101); } - public function testGetUrlValid() { + public function testGetUrlValid(): void { $userFolder = $this->createMock(Folder::class); $this->rootFolder->method('getUserFolder') ->with('awesomeUser') @@ -128,6 +133,9 @@ class DirectControllerTest extends TestCase { ->with(101) ->willReturn([$file]); + $userFolder->method('getRelativePath') + ->willReturn('/path'); + $this->random->method('generate') ->with( 60, diff --git a/apps/dav/tests/unit/DAV/ViewOnlyPluginTest.php b/apps/dav/tests/unit/DAV/ViewOnlyPluginTest.php new file mode 100644 index 00000000000..f86a60fb4bf --- /dev/null +++ b/apps/dav/tests/unit/DAV/ViewOnlyPluginTest.php @@ -0,0 +1,117 @@ +<?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\File; +use OCP\Files\Storage\IStorage; +use OCP\Share\IAttributes; +use OCP\Share\IShare; +use Psr\Log\LoggerInterface; +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 { + + private ViewOnlyPlugin $plugin; + /** @var Tree | \PHPUnit\Framework\MockObject\MockObject */ + private $tree; + /** @var RequestInterface | \PHPUnit\Framework\MockObject\MockObject */ + private $request; + + public function setUp(): void { + $this->plugin = new ViewOnlyPlugin( + $this->createMock(LoggerInterface::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(): void { + $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 testCanGetNonShared(): void { + $this->request->expects($this->once())->method('getPath')->willReturn('files/test/target'); + $davNode = $this->createMock(DavFile::class); + $this->tree->method('getNodeForPath')->willReturn($davNode); + + $file = $this->createMock(File::class); + $davNode->method('getNode')->willReturn($file); + + $storage = $this->createMock(IStorage::class); + $file->method('getStorage')->willReturn($storage); + $storage->method('instanceOfStorage')->with(SharedStorage::class)->willReturn(false); + + $this->assertTrue($this->plugin->checkViewOnly($this->request)); + } + + public function providesDataForCanGet(): array { + return [ + // has attribute permissions-download enabled - can get file + [ $this->createMock(File::class), true, true], + // has no attribute permissions-download - can get file + [ $this->createMock(File::class), null, true], + // has attribute permissions-download disabled- cannot get the file + [ $this->createMock(File::class), false, false], + ]; + } + + /** + * @dataProvider providesDataForCanGet + */ + public function testCanGet(File $nodeInfo, ?bool $attrEnabled, bool $expectCanDownloadFile): void { + $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); + } +} diff --git a/apps/files/js/fileinfomodel.js b/apps/files/js/fileinfomodel.js index 8e7b399544c..83a8c62592b 100644 --- a/apps/files/js/fileinfomodel.js +++ b/apps/files/js/fileinfomodel.js @@ -84,6 +84,15 @@ }, /** + * 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 * @return ajax call object diff --git a/apps/files/src/services/FileInfo.js b/apps/files/src/services/FileInfo.js index 8b62063e134..c09af45f495 100644 --- a/apps/files/src/services/FileInfo.js +++ b/apps/files/src/services/FileInfo.js @@ -47,6 +47,7 @@ export default async function(url) { <nc:mount-type /> <nc:is-encrypted /> <ocs:share-permissions /> + <nc:share-attributes /> <oc:tags /> <oc:favorite /> <oc:comments-unread /> diff --git a/apps/files_external/lib/Lib/Storage/AmazonS3.php b/apps/files_external/lib/Lib/Storage/AmazonS3.php index cfd78689fa4..62bff08895b 100644 --- a/apps/files_external/lib/Lib/Storage/AmazonS3.php +++ b/apps/files_external/lib/Lib/Storage/AmazonS3.php @@ -747,7 +747,7 @@ class AmazonS3 extends \OC\Files\Storage\Common { return $result->get('Status') === 'Enabled'; } catch (S3Exception $s3Exception) { // This is needed for compatibility with Storj gateway which does not support versioning yet - if ($s3Exception->getAwsErrorCode() === 'NotImplemented') { + if ($s3Exception->getAwsErrorCode() === 'NotImplemented' || $s3Exception->getAwsErrorCode() === 'AccessDenied') { return false; } throw $s3Exception; diff --git a/apps/files_sharing/composer/composer/autoload_classmap.php b/apps/files_sharing/composer/composer/autoload_classmap.php index 2810910c8c9..e4a493cadfb 100644 --- a/apps/files_sharing/composer/composer/autoload_classmap.php +++ b/apps/files_sharing/composer/composer/autoload_classmap.php @@ -80,4 +80,5 @@ return array( 'OCA\\Files_Sharing\\SharedMount' => $baseDir . '/../lib/SharedMount.php', 'OCA\\Files_Sharing\\SharedStorage' => $baseDir . '/../lib/SharedStorage.php', 'OCA\\Files_Sharing\\Updater' => $baseDir . '/../lib/Updater.php', + 'OCA\\Files_Sharing\\ViewOnly' => $baseDir . '/../lib/ViewOnly.php', ); diff --git a/apps/files_sharing/composer/composer/autoload_static.php b/apps/files_sharing/composer/composer/autoload_static.php index 70149b1cdc0..3c92a46fc82 100644 --- a/apps/files_sharing/composer/composer/autoload_static.php +++ b/apps/files_sharing/composer/composer/autoload_static.php @@ -95,6 +95,7 @@ class ComposerStaticInitFiles_Sharing 'OCA\\Files_Sharing\\SharedMount' => __DIR__ . '/..' . '/../lib/SharedMount.php', 'OCA\\Files_Sharing\\SharedStorage' => __DIR__ . '/..' . '/../lib/SharedStorage.php', 'OCA\\Files_Sharing\\Updater' => __DIR__ . '/..' . '/../lib/Updater.php', + 'OCA\\Files_Sharing\\ViewOnly' => __DIR__ . '/..' . '/../lib/ViewOnly.php', ); public static function getInitializer(ClassLoader $loader) diff --git a/apps/files_sharing/lib/AppInfo/Application.php b/apps/files_sharing/lib/AppInfo/Application.php index 2539247b561..4bf1ef5a843 100644 --- a/apps/files_sharing/lib/AppInfo/Application.php +++ b/apps/files_sharing/lib/AppInfo/Application.php @@ -50,15 +50,21 @@ use OCA\Files_Sharing\Notification\Listener; use OCA\Files_Sharing\Notification\Notifier; use OCA\Files\Event\LoadAdditionalScriptsEvent; use OCA\Files\Event\LoadSidebar; +use OCP\Files\Event\BeforeDirectGetEvent; use OCA\Files_Sharing\ShareBackend\File; use OCA\Files_Sharing\ShareBackend\Folder; +use OCA\Files_Sharing\ViewOnly; use OCP\AppFramework\App; use OCP\AppFramework\Bootstrap\IBootContext; use OCP\AppFramework\Bootstrap\IBootstrap; use OCP\AppFramework\Bootstrap\IRegistrationContext; use OCP\EventDispatcher\IEventDispatcher; +use OCP\EventDispatcher\GenericEvent; use OCP\Federation\ICloudIdManager; use OCP\Files\Config\IMountProviderCollection; +use OCP\Files\Events\BeforeDirectFileDownloadEvent; +use OCP\Files\Events\BeforeZipCreatedEvent; +use OCP\Files\IRootFolder; use OCP\Group\Events\UserAddedEvent; use OCP\IDBConnection; use OCP\IGroup; @@ -70,7 +76,7 @@ use OCP\User\Events\UserChangedEvent; use OCP\Util; use Psr\Container\ContainerInterface; use Symfony\Component\EventDispatcher\EventDispatcherInterface; -use Symfony\Component\EventDispatcher\GenericEvent; +use Symfony\Component\EventDispatcher\GenericEvent as OldGenericEvent; class Application extends App implements IBootstrap { public const APP_ID = 'files_sharing'; @@ -106,6 +112,7 @@ class Application extends App implements IBootstrap { public function boot(IBootContext $context): void { $context->injectFn([$this, 'registerMountProviders']); $context->injectFn([$this, 'registerEventsScripts']); + $context->injectFn([$this, 'registerDownloadEvents']); $context->injectFn([$this, 'setupSharingMenus']); Helper::registerHooks(); @@ -120,12 +127,12 @@ class Application extends App implements IBootstrap { } - public function registerMountProviders(IMountProviderCollection $mountProviderCollection, MountProvider $mountProvider, ExternalMountProvider $externalMountProvider) { + public function registerMountProviders(IMountProviderCollection $mountProviderCollection, MountProvider $mountProvider, ExternalMountProvider $externalMountProvider): void { $mountProviderCollection->registerProvider($mountProvider); $mountProviderCollection->registerProvider($externalMountProvider); } - public function registerEventsScripts(IEventDispatcher $dispatcher, EventDispatcherInterface $oldDispatcher) { + public function registerEventsScripts(IEventDispatcher $dispatcher, EventDispatcherInterface $oldDispatcher): void { // sidebar and files scripts $dispatcher->addServiceListener(LoadAdditionalScriptsEvent::class, LoadAdditionalListener::class); $dispatcher->addServiceListener(BeforeTemplateRenderedEvent::class, LegacyBeforeTemplateRenderedListener::class); @@ -138,19 +145,73 @@ class Application extends App implements IBootstrap { $dispatcher->addServiceListener(UserAddedEvent::class, UserAddedToGroupListener::class); // notifications api to accept incoming user shares - $oldDispatcher->addListener('OCP\Share::postShare', function (GenericEvent $event) { + $oldDispatcher->addListener('OCP\Share::postShare', function (OldGenericEvent $event) { /** @var Listener $listener */ $listener = $this->getContainer()->query(Listener::class); $listener->shareNotification($event); }); - $oldDispatcher->addListener(IGroup::class . '::postAddUser', function (GenericEvent $event) { + $oldDispatcher->addListener(IGroup::class . '::postAddUser', function (OldGenericEvent $event) { /** @var Listener $listener */ $listener = $this->getContainer()->query(Listener::class); $listener->userAddedToGroup($event); }); } - public function setupSharingMenus(IManager $shareManager, IFactory $l10nFactory, IUserSession $userSession) { + public function registerDownloadEvents( + IEventDispatcher $dispatcher, + IUserSession $userSession, + IRootFolder $rootFolder + ): void { + + $dispatcher->addListener( + BeforeDirectFileDownloadEvent::class, + function (BeforeDirectFileDownloadEvent $event) use ($userSession, $rootFolder): void { + $pathsToCheck = [$event->getPath()]; + // Check only for user/group shares. Don't restrict e.g. share links + $user = $userSession->getUser(); + if ($user) { + $viewOnlyHandler = new ViewOnly( + $rootFolder->getUserFolder($user->getUID()) + ); + if (!$viewOnlyHandler->check($pathsToCheck)) { + $event->setSuccessful(false); + $event->setErrorMessage('Access to this resource or one of its sub-items has been denied.'); + } + } + } + ); + + $dispatcher->addListener( + BeforeZipCreatedEvent::class, + function (BeforeZipCreatedEvent $event) use ($userSession, $rootFolder): void { + $dir = $event->getDirectory(); + $files = $event->getFiles(); + + $pathsToCheck = []; + foreach ($files as $file) { + $pathsToCheck[] = $dir . '/' . $file; + } + + // Check only for user/group shares. Don't restrict e.g. share links + $user = $userSession->getUser(); + if ($user) { + $viewOnlyHandler = new ViewOnly( + $rootFolder->getUserFolder($user->getUID()) + ); + if (!$viewOnlyHandler->check($pathsToCheck)) { + $event->setErrorMessage('Access to this resource or one of its sub-items has been denied.'); + $event->setSuccessful(false); + } else { + $event->setSuccessful(true); + } + } else { + $event->setSuccessful(true); + } + } + ); + } + + public function setupSharingMenus(IManager $shareManager, IFactory $l10nFactory, IUserSession $userSession): void { if (!$shareManager->shareApiEnabled() || !class_exists('\OCA\Files\App')) { return; } diff --git a/apps/files_sharing/lib/Controller/PublicPreviewController.php b/apps/files_sharing/lib/Controller/PublicPreviewController.php index 4a16afa7ac0..98c4d8cafb4 100644 --- a/apps/files_sharing/lib/Controller/PublicPreviewController.php +++ b/apps/files_sharing/lib/Controller/PublicPreviewController.php @@ -136,7 +136,7 @@ class PublicPreviewController extends PublicShareController { * @param $token * @return DataResponse|FileDisplayResponse */ - public function directLink($token) { + public function directLink(string $token) { // No token no image if ($token === '') { return new DataResponse([], Http::STATUS_BAD_REQUEST); diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index f531954d5da..66b3153f099 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -45,8 +45,10 @@ declare(strict_types=1); namespace OCA\Files_Sharing\Controller; use OC\Files\FileInfo; +use OC\Files\Storage\Wrapper\Wrapper; use OCA\Files_Sharing\Exceptions\SharingRightsException; use OCA\Files_Sharing\External\Storage; +use OCA\Files_Sharing\SharedStorage; use OCA\Files\Helper; use OCP\App\IAppManager; use OCP\AppFramework\Http\DataResponse; @@ -324,6 +326,11 @@ class ShareAPIController extends OCSController { $result['mail_send'] = $share->getMailSend() ? 1 : 0; $result['hide_download'] = $share->getHideDownload() ? 1 : 0; + $result['attributes'] = null; + if ($attributes = $share->getAttributes()) { + $result['attributes'] = \json_encode($attributes->toArray()); + } + return $result; } @@ -436,6 +443,7 @@ class ShareAPIController extends OCSController { * @param string $sendPasswordByTalk * @param string $expireDate * @param string $label + * @param string $attributes * * @return DataResponse * @throws NotFoundException @@ -456,7 +464,8 @@ class ShareAPIController extends OCSController { string $sendPasswordByTalk = null, string $expireDate = '', string $note = '', - string $label = '' + string $label = '', + string $attributes = null ): DataResponse { $share = $this->shareManager->newShare(); @@ -516,6 +525,8 @@ class ShareAPIController extends OCSController { $permissions &= ~($permissions & ~$node->getPermissions()); } + $this->checkInheritedAttributes($share); + if ($shareType === IShare::TYPE_USER) { // Valid user is required to share if ($shareWith === null || !$this->userManager->userExists($shareWith)) { @@ -674,6 +685,10 @@ class ShareAPIController extends OCSController { $share->setNote($note); } + if ($attributes !== null) { + $share = $this->setShareAttributes($share, $attributes); + } + try { $share = $this->shareManager->createShare($share); } catch (GenericShareException $e) { @@ -1035,6 +1050,7 @@ class ShareAPIController extends OCSController { * @param string $note * @param string $label * @param string $hideDownload + * @param string $attributes * @return DataResponse * @throws LockedException * @throws NotFoundException @@ -1051,7 +1067,8 @@ class ShareAPIController extends OCSController { string $expireDate = null, string $note = null, string $label = null, - string $hideDownload = null + string $hideDownload = null, + string $attributes = null ): DataResponse { try { $share = $this->getShareById($id); @@ -1077,7 +1094,8 @@ class ShareAPIController extends OCSController { $expireDate === null && $note === null && $label === null && - $hideDownload === null + $hideDownload === null && + $attributes === null ) { throw new OCSBadRequestException($this->l->t('Wrong or no update parameter given')); } @@ -1086,6 +1104,25 @@ class ShareAPIController extends OCSController { $share->setNote($note); } + $userFolder = $this->rootFolder->getUserFolder($this->currentUser); + + // get the node with the point of view of the current user + $nodes = $userFolder->getById($share->getNode()->getId()); + if (count($nodes) > 0) { + $node = $nodes[0]; + $storage = $node->getStorage(); + if ($storage && $storage->instanceOfStorage(SharedStorage::class)) { + /** @var \OCA\Files_Sharing\SharedStorage $storage */ + $inheritedAttributes = $storage->getShare()->getAttributes(); + if ($inheritedAttributes !== null && $inheritedAttributes->getAttribute('permissions', 'download') === false) { + if ($hideDownload === 'false') { + throw new OCSBadRequestException($this->l->t('Cannot increase permissions')); + } + $share->setHideDownload(true); + } + } + } + /** * expirationdate, password and publicUpload only make sense for link shares */ @@ -1216,6 +1253,10 @@ class ShareAPIController extends OCSController { } } + if ($attributes !== null) { + $share = $this->setShareAttributes($share, $attributes); + } + try { $share = $this->shareManager->updateShare($share); } catch (GenericShareException $e) { @@ -1832,4 +1873,51 @@ class ShareAPIController extends OCSController { } } } + + /** + * @param IShare $share + * @param string|null $attributesString + * @return IShare modified share + */ + private function setShareAttributes(IShare $share, ?string $attributesString) { + $newShareAttributes = null; + if ($attributesString !== null) { + $newShareAttributes = $this->shareManager->newShare()->newAttributes(); + $formattedShareAttributes = \json_decode($attributesString, true); + if (is_array($formattedShareAttributes)) { + foreach ($formattedShareAttributes as $formattedAttr) { + $newShareAttributes->setAttribute( + $formattedAttr['scope'], + $formattedAttr['key'], + is_string($formattedAttr['enabled']) ? (bool) \json_decode($formattedAttr['enabled']) : $formattedAttr['enabled'] + ); + } + } else { + throw new OCSBadRequestException('Invalid share attributes provided: \"' . $attributesString . '\"'); + } + } + $share->setAttributes($newShareAttributes); + + return $share; + } + + private function checkInheritedAttributes(IShare $share): void { + if ($share->getNode()->getStorage()->instanceOfStorage(SharedStorage::class)) { + $storage = $share->getNode()->getStorage(); + if ($storage instanceof Wrapper) { + $storage = $storage->getInstanceOfStorage(SharedStorage::class); + if ($storage === null) { + throw new \RuntimeException('Should not happen, instanceOfStorage but getInstanceOfStorage return null'); + } + } else { + throw new \RuntimeException('Should not happen, instanceOfStorage but not a wrapper'); + } + /** @var \OCA\Files_Sharing\SharedStorage $storage */ + $inheritedAttributes = $storage->getShare()->getAttributes(); + if ($inheritedAttributes !== null && $inheritedAttributes->getAttribute('permissions', 'download') === false) { + $share->setHideDownload(true); + } + } + + } } diff --git a/apps/files_sharing/lib/External/Scanner.php b/apps/files_sharing/lib/External/Scanner.php index cfde56103da..e3336c69a05 100644 --- a/apps/files_sharing/lib/External/Scanner.php +++ b/apps/files_sharing/lib/External/Scanner.php @@ -29,27 +29,14 @@ use OC\ForbiddenException; use OCP\Files\NotFoundException; use OCP\Files\StorageInvalidException; use OCP\Files\StorageNotAvailableException; -use OCP\Http\Client\LocalServerException; -use Psr\Log\LoggerInterface; class Scanner extends \OC\Files\Cache\Scanner { /** @var \OCA\Files_Sharing\External\Storage */ protected $storage; - /** {@inheritDoc} */ public function scan($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $lock = true) { - try { - if (!$this->storage->remoteIsOwnCloud()) { - return parent::scan($path, $recursive, $reuse, $lock); - } - } catch (LocalServerException $e) { - // Scanner doesn't have dependency injection - \OC::$server->get(LoggerInterface::class) - ->warning('Trying to scan files inside invalid external storage: ' . $this->storage->getRemote() . ' for mountpoint ' . $this->storage->getMountPoint() . ' and id ' . $this->storage->getId()); - return; - } - - $this->scanAll(); + // Disable locking for federated shares + parent::scan($path, $recursive, $reuse, false); } /** @@ -67,7 +54,7 @@ class Scanner extends \OC\Files\Cache\Scanner { */ public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData = null, $lock = true, $data = null) { try { - return parent::scanFile($file, $reuseExisting); + return parent::scanFile($file, $reuseExisting, $parentId, $cacheData, $lock, $data); } catch (ForbiddenException $e) { $this->storage->checkStorageAvailability(); } catch (NotFoundException $e) { @@ -81,56 +68,4 @@ class Scanner extends \OC\Files\Cache\Scanner { $this->storage->checkStorageAvailability(); } } - - /** - * Checks the remote share for changes. - * If changes are available, scan them and update - * the cache. - * @throws NotFoundException - * @throws StorageInvalidException - * @throws \Exception - */ - public function scanAll() { - try { - $data = $this->storage->getShareInfo(); - } catch (\Exception $e) { - $this->storage->checkStorageAvailability(); - throw new \Exception( - 'Error while scanning remote share: "' . - $this->storage->getRemote() . '" ' . - $e->getMessage() - ); - } - if ($data['status'] === 'success') { - $this->addResult($data['data'], ''); - } else { - throw new \Exception( - 'Error while scanning remote share: "' . - $this->storage->getRemote() . '"' - ); - } - } - - /** - * @param array $data - * @param string $path - */ - private function addResult($data, $path) { - $id = $this->cache->put($path, $data); - if (isset($data['children'])) { - $children = []; - foreach ($data['children'] as $child) { - $children[$child['name']] = true; - $this->addResult($child, ltrim($path . '/' . $child['name'], '/')); - } - - $existingCache = $this->cache->getFolderContentsById($id); - foreach ($existingCache as $existingChild) { - // if an existing child is not in the new data, remove it - if (!isset($children[$existingChild['name']])) { - $this->cache->remove(ltrim($path . '/' . $existingChild['name'], '/')); - } - } - } - } } diff --git a/apps/files_sharing/lib/MountProvider.php b/apps/files_sharing/lib/MountProvider.php index bfb40387622..d85d796c722 100644 --- a/apps/files_sharing/lib/MountProvider.php +++ b/apps/files_sharing/lib/MountProvider.php @@ -38,6 +38,7 @@ use OCP\ICacheFactory; use OCP\IConfig; use OCP\ILogger; use OCP\IUser; +use OCP\Share\IAttributes; use OCP\Share\IManager; use OCP\Share\IShare; @@ -228,14 +229,32 @@ class MountProvider implements IMountProvider { ->setTarget($shares[0]->getTarget()); // use most permissive permissions - $permissions = 0; + // this covers the case where there are multiple shares for the same + // file e.g. from different groups and different permissions + $superPermissions = 0; + $superAttributes = $this->shareManager->newShare()->newAttributes(); $status = IShare::STATUS_PENDING; foreach ($shares as $share) { - $permissions |= $share->getPermissions(); + $superPermissions |= $share->getPermissions(); $status = max($status, $share->getStatus()); + // update permissions + $superPermissions |= $share->getPermissions(); + + // update share permission attributes + $attributes = $share->getAttributes(); + if ($attributes !== null) { + foreach ($attributes->toArray() as $attribute) { + if ($superAttributes->getAttribute($attribute['scope'], $attribute['key']) === true) { + // if super share attribute is already enabled, it is most permissive + continue; + } + // update supershare attributes with subshare attribute + $superAttributes->setAttribute($attribute['scope'], $attribute['key'], $attribute['enabled']); + } + } + // adjust target, for database consistency if needed if ($share->getTarget() !== $superShare->getTarget()) { - // adjust target, for database consistency $share->setTarget($superShare->getTarget()); try { $this->shareManager->moveShare($share, $user->getUID()); @@ -260,8 +279,9 @@ class MountProvider implements IMountProvider { } } - $superShare->setPermissions($permissions) - ->setStatus($status); + $superShare->setPermissions($superPermissions); + $superShare->setStatus($status); + $superShare->setAttributes($superAttributes); $result[] = [$superShare, $shares]; } diff --git a/apps/files_sharing/lib/ViewOnly.php b/apps/files_sharing/lib/ViewOnly.php new file mode 100644 index 00000000000..26e8e43a871 --- /dev/null +++ b/apps/files_sharing/lib/ViewOnly.php @@ -0,0 +1,121 @@ +<?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\Files_Sharing; + +use OCP\Files\File; +use OCP\Files\Folder; +use OCP\Files\Node; +use OCP\Files\NotFoundException; + +/** + * Handles restricting for download of files + */ +class ViewOnly { + + /** @var Folder */ + private $userFolder; + + public function __construct(Folder $userFolder) { + $this->userFolder = $userFolder; + } + + /** + * @param string[] $pathsToCheck + * @return bool + */ + public function check(array $pathsToCheck): bool { + // If any of elements cannot be downloaded, prevent whole download + foreach ($pathsToCheck as $file) { + try { + $info = $this->userFolder->get($file); + if ($info instanceof File) { + // access to filecache is expensive in the loop + if (!$this->checkFileInfo($info)) { + return false; + } + } elseif ($info instanceof Folder) { + // get directory content is rather cheap query + if (!$this->dirRecursiveCheck($info)) { + return false; + } + } + } catch (NotFoundException $e) { + continue; + } + } + return true; + } + + /** + * @param Folder $dirInfo + * @return bool + * @throws NotFoundException + */ + private function dirRecursiveCheck(Folder $dirInfo): bool { + if (!$this->checkFileInfo($dirInfo)) { + return false; + } + // If any of elements cannot be downloaded, prevent whole download + $files = $dirInfo->getDirectoryListing(); + foreach ($files as $file) { + if ($file instanceof File) { + if (!$this->checkFileInfo($file)) { + return false; + } + } elseif ($file instanceof Folder) { + return $this->dirRecursiveCheck($file); + } + } + + return true; + } + + /** + * @param Node $fileInfo + * @return bool + * @throws NotFoundException + */ + private function checkFileInfo(Node $fileInfo): bool { + // Restrict view-only to nodes which are shared + $storage = $fileInfo->getStorage(); + if (!$storage->instanceOfStorage(SharedStorage::class)) { + return true; + } + + // Extract extra permissions + /** @var \OCA\Files_Sharing\SharedStorage $storage */ + $share = $storage->getShare(); + + $canDownload = true; + + // Check if read-only and on whether permission can download is both set and disabled. + $attributes = $share->getAttributes(); + if ($attributes !== null) { + $canDownload = $attributes->getAttribute('permissions', 'download'); + } + + if ($canDownload !== null && !$canDownload) { + return false; + } + return true; + } +} diff --git a/apps/files_sharing/src/components/SharingEntry.vue b/apps/files_sharing/src/components/SharingEntry.vue index 2d29aaf1dc6..e06a8acef3b 100644 --- a/apps/files_sharing/src/components/SharingEntry.vue +++ b/apps/files_sharing/src/components/SharingEntry.vue @@ -78,6 +78,13 @@ {{ t('files_sharing', 'Allow resharing') }} </ActionCheckbox> + <ActionCheckbox ref="canDownload" + :checked.sync="canDownload" + v-if="isSetDownloadButtonVisible" + :disabled="saving || !canSetDownload"> + {{ allowDownloadText }} + </ActionCheckbox> + <!-- expiration date --> <ActionCheckbox :checked.sync="hasExpirationDate" :disabled="config.isDefaultInternalExpireDateEnforced || saving" @@ -272,6 +279,18 @@ export default { }, /** + * Can the sharer set whether the sharee can download the file ? + * + * @return {boolean} + */ + canSetDownload() { + // If the owner revoked the permission after the resharer granted it + // the share still has the permission, and the resharer is still + // allowed to revoke it too (but not to grant it again). + return (this.fileInfo.canDownload() || this.canDownload) + }, + + /** * Can the sharee edit the shared file ? */ canEdit: { @@ -320,6 +339,18 @@ export default { }, /** + * Can the sharee download files or only view them ? + */ + canDownload: { + get() { + return this.share.hasDownloadPermission + }, + set(checked) { + this.updatePermissions({ isDownloadChecked: checked }) + }, + }, + + /** * Is this share readable * Needed for some federated shares that might have been added from file drop links */ @@ -377,10 +408,42 @@ export default { return (typeof this.share.status === 'object' && !Array.isArray(this.share.status)) }, + /** + * @return {string} + */ + allowDownloadText() { + return t('files_sharing', 'Allow download') + }, + + /** + * @return {boolean} + */ + isSetDownloadButtonVisible() { + const allowedMimetypes = [ + // Office documents + 'application/msword', + 'application/vnd.openxmlformats-officedocument.wordprocessingml.document', + 'application/vnd.ms-powerpoint', + 'application/vnd.openxmlformats-officedocument.presentationml.presentation', + 'application/vnd.ms-excel', + 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', + 'application/vnd.oasis.opendocument.text', + 'application/vnd.oasis.opendocument.spreadsheet', + 'application/vnd.oasis.opendocument.presentation', + ] + + return this.isFolder || allowedMimetypes.includes(this.fileInfo.mimetype) + }, }, methods: { - updatePermissions({ isEditChecked = this.canEdit, isCreateChecked = this.canCreate, isDeleteChecked = this.canDelete, isReshareChecked = this.canReshare } = {}) { + updatePermissions({ + isEditChecked = this.canEdit, + isCreateChecked = this.canCreate, + isDeleteChecked = this.canDelete, + isReshareChecked = this.canReshare, + isDownloadChecked = this.canDownload, + } = {}) { // calc permissions if checked const permissions = 0 | (this.hasRead ? this.permissionsRead : 0) @@ -390,7 +453,10 @@ export default { | (isReshareChecked ? this.permissionsShare : 0) this.share.permissions = permissions - this.queueUpdate('permissions') + if (this.share.hasDownloadPermission !== isDownloadChecked) { + this.share.hasDownloadPermission = isDownloadChecked + } + this.queueUpdate('permissions', 'attributes') }, /** diff --git a/apps/files_sharing/src/components/SharingEntryLink.vue b/apps/files_sharing/src/components/SharingEntryLink.vue index 638cdf485b0..2dc10de5484 100644 --- a/apps/files_sharing/src/components/SharingEntryLink.vue +++ b/apps/files_sharing/src/components/SharingEntryLink.vue @@ -158,7 +158,7 @@ <ActionSeparator /> <ActionCheckbox :checked.sync="share.hideDownload" - :disabled="saving" + :disabled="saving || canChangeHideDownload" @change="queueUpdate('hideDownload')"> {{ t('files_sharing', 'Hide download') }} </ActionCheckbox> @@ -606,6 +606,12 @@ export default { isPasswordPolicyEnabled() { return typeof this.config.passwordPolicy === 'object' }, + + canChangeHideDownload() { + const hasDisabledDownload = (shareAttribute) => shareAttribute.key === 'download' && shareAttribute.scope === 'permissions' && shareAttribute.enabled === false + + return this.fileInfo.shareAttributes.some(hasDisabledDownload) + }, }, methods: { @@ -696,6 +702,7 @@ export default { shareType: ShareTypes.SHARE_TYPE_LINK, password: share.password, expireDate: share.expireDate, + attributes: JSON.stringify(this.fileInfo.shareAttributes), // we do not allow setting the publicUpload // before the share creation. // Todo: We also need to fix the createShare method in @@ -866,7 +873,6 @@ export default { this.$emit('remove:share', this.share) }, }, - } </script> diff --git a/apps/files_sharing/src/components/SharingInput.vue b/apps/files_sharing/src/components/SharingInput.vue index 02c1f27f173..716e96a4e06 100644 --- a/apps/files_sharing/src/components/SharingInput.vue +++ b/apps/files_sharing/src/components/SharingInput.vue @@ -478,6 +478,7 @@ export default { shareWith: value.shareWith, password, permissions: this.fileInfo.sharePermissions & OC.getCapabilities().files_sharing.default_permissions, + attributes: JSON.stringify(this.fileInfo.shareAttributes), }) // If we had a password, we need to show it to the user as it was generated diff --git a/apps/files_sharing/src/mixins/ShareRequests.js b/apps/files_sharing/src/mixins/ShareRequests.js index e2668c15d65..9eaad8c4161 100644 --- a/apps/files_sharing/src/mixins/ShareRequests.js +++ b/apps/files_sharing/src/mixins/ShareRequests.js @@ -47,12 +47,13 @@ export default { * @param {boolean} [data.sendPasswordByTalk=false] send the password via a talk conversation * @param {string} [data.expireDate=''] expire the shareautomatically after * @param {string} [data.label=''] custom label + * @param {string} [data.attributes=null] Share attributes encoded as json * @return {Share} the new share * @throws {Error} */ - async createShare({ path, permissions, shareType, shareWith, publicUpload, password, sendPasswordByTalk, expireDate, label }) { + async createShare({ path, permissions, shareType, shareWith, publicUpload, password, sendPasswordByTalk, expireDate, label, attributes }) { try { - const request = await axios.post(shareUrl, { path, permissions, shareType, shareWith, publicUpload, password, sendPasswordByTalk, expireDate, label }) + const request = await axios.post(shareUrl, { path, permissions, shareType, shareWith, publicUpload, password, sendPasswordByTalk, expireDate, label, attributes }) if (!request?.data?.ocs) { throw request } diff --git a/apps/files_sharing/src/mixins/SharesMixin.js b/apps/files_sharing/src/mixins/SharesMixin.js index daeacfa4b8b..053babd3a1d 100644 --- a/apps/files_sharing/src/mixins/SharesMixin.js +++ b/apps/files_sharing/src/mixins/SharesMixin.js @@ -229,7 +229,13 @@ export default { const properties = {} // force value to string because that is what our // share api controller accepts - propertyNames.map(p => (properties[p] = this.share[p].toString())) + propertyNames.forEach(name => { + if ((typeof this.share[name]) === 'object') { + properties[name] = JSON.stringify(this.share[name]) + } else { + properties[name] = this.share[name].toString() + } + }) this.updateQueue.add(async () => { this.saving = true diff --git a/apps/files_sharing/src/models/Share.js b/apps/files_sharing/src/models/Share.js index 5644ce0c2b3..e6512c67f8c 100644 --- a/apps/files_sharing/src/models/Share.js +++ b/apps/files_sharing/src/models/Share.js @@ -43,6 +43,15 @@ export default class Share { ocsData.hide_download = !!ocsData.hide_download ocsData.mail_send = !!ocsData.mail_send + if (ocsData.attributes) { + try { + ocsData.attributes = JSON.parse(ocsData.attributes) + } catch (e) { + console.warn('Could not parse share attributes returned by server: "' + ocsData.attributes + '"') + } + } + ocsData.attributes = ocsData.attributes ?? [] + // store state this._share = ocsData } @@ -97,6 +106,17 @@ export default class Share { } /** + * Get the share attributes + * + * @return {Array} + * @readonly + * @memberof Share + */ + get attributes() { + return this._share.attributes + } + + /** * Set the share permissions * See OC.PERMISSION_* variables * @@ -527,6 +547,47 @@ export default class Share { return !!((this.permissions & OC.PERMISSION_SHARE)) } + /** + * Does this share have download permissions + * + * @return {boolean} + * @readonly + * @memberof Share + */ + get hasDownloadPermission() { + for (const i in this._share.attributes) { + const attr = this._share.attributes[i] + if (attr.scope === 'permissions' && attr.key === 'download') { + return attr.enabled + } + } + + return true + } + + set hasDownloadPermission(enabled) { + this.setAttribute('permissions', 'download', !!enabled) + } + + setAttribute(scope, key, enabled) { + const attrUpdate = { + scope, + key, + enabled, + } + + // try and replace existing + for (const i in this._share.attributes) { + const attr = this._share.attributes[i] + if (attr.scope === attrUpdate.scope && attr.key === attrUpdate.key) { + this._share.attributes[i] = attrUpdate + return + } + } + + this._share.attributes.push(attrUpdate) + } + // PERMISSIONS Shortcuts for the CURRENT USER // ! the permissions above are the share settings, // ! meaning the permissions for the recipient diff --git a/apps/files_sharing/src/share.js b/apps/files_sharing/src/share.js index c533e7b8109..76c007b5218 100644 --- a/apps/files_sharing/src/share.js +++ b/apps/files_sharing/src/share.js @@ -92,7 +92,11 @@ import { getCapabilities } from '@nextcloud/capabilities' delete fileActions.actions.all.Details delete fileActions.actions.all.Goto } + if (_.isFunction(fileData.canDownload) && !fileData.canDownload()) { + delete fileActions.actions.all.Download + } tr.attr('data-share-permissions', sharePermissions) + tr.attr('data-share-attributes', JSON.stringify(fileData.shareAttributes)) if (fileData.shareOwner) { tr.attr('data-share-owner', fileData.shareOwner) tr.attr('data-share-owner-id', fileData.shareOwnerId) @@ -113,6 +117,7 @@ import { getCapabilities } from '@nextcloud/capabilities' var oldElementToFile = fileList.elementToFile fileList.elementToFile = function($el) { var fileInfo = oldElementToFile.apply(this, arguments) + fileInfo.shareAttributes = JSON.parse($el.attr('data-share-attributes') || '[]') fileInfo.sharePermissions = $el.attr('data-share-permissions') || undefined fileInfo.shareOwner = $el.attr('data-share-owner') || undefined fileInfo.shareOwnerId = $el.attr('data-share-owner-id') || undefined diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php index c7159e82163..1bc75361dfc 100644 --- a/apps/files_sharing/tests/ApiTest.php +++ b/apps/files_sharing/tests/ApiTest.php @@ -948,8 +948,15 @@ class ApiTest extends TestCase { ->setSharedBy(self::TEST_FILES_SHARING_API_USER1) ->setSharedWith(self::TEST_FILES_SHARING_API_USER2) ->setShareType(IShare::TYPE_USER) - ->setPermissions(19); + ->setPermissions(19) + ->setAttributes($this->shareManager->newShare()->newAttributes()); + + $this->assertNotNull($share1->getAttributes()); $share1 = $this->shareManager->createShare($share1); + $this->assertNull($share1->getAttributes()); + $this->assertEquals(19, $share1->getPermissions()); + // attributes get cleared when empty + $this->assertNull($share1->getAttributes()); $share2 = $this->shareManager->newShare(); $share2->setNode($node1) @@ -957,14 +964,19 @@ class ApiTest extends TestCase { ->setShareType(IShare::TYPE_LINK) ->setPermissions(1); $share2 = $this->shareManager->createShare($share2); + $this->assertEquals(1, $share2->getPermissions()); // update permissions $ocs = $this->createOCS(self::TEST_FILES_SHARING_API_USER1); - $ocs->updateShare($share1->getId(), 1); + $ocs->updateShare( + $share1->getId(), 1, null, null, null, null, null, null, null, + '[{"scope": "app1", "key": "attr1", "enabled": true}]' + ); $ocs->cleanup(); $share1 = $this->shareManager->getShareById('ocinternal:' . $share1->getId()); $this->assertEquals(1, $share1->getPermissions()); + $this->assertEquals(true, $share1->getAttributes()->getAttribute('app1', 'attr1')); // update password for link share $this->assertNull($share2->getPassword()); diff --git a/apps/files_sharing/tests/ApplicationTest.php b/apps/files_sharing/tests/ApplicationTest.php new file mode 100644 index 00000000000..11c8137c398 --- /dev/null +++ b/apps/files_sharing/tests/ApplicationTest.php @@ -0,0 +1,236 @@ +<?php +/** + * @copyright 2022, Vincent Petry <vincent@nextcloud.com> + * + * @author Vincent Petry <vincent@nextcloud.com> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * 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 + * along with this program. If not, see <http://www.gnu.org/licenses/>. + * + */ +namespace OCA\Files_Sharing\Tests; + +use OCP\Files\Events\BeforeDirectFileDownloadEvent; +use OCP\Files\Events\BeforeZipCreatedEvent; +use Psr\Log\LoggerInterface; +use OC\Share20\LegacyHooks; +use OC\Share20\Manager; +use OC\EventDispatcher\EventDispatcher; +use OCA\Files_Sharing\AppInfo\Application; +use OCA\Files_Sharing\SharedStorage; +use OCP\Constants; +use OCP\EventDispatcher\GenericEvent; +use OCP\EventDispatcher\IEventDispatcher; +use OCP\Files\Cache\ICacheEntry; +use OCP\Files\Event\BeforeDirectGetEvent; +use OCP\Files\File; +use OCP\Files\Folder; +use OCP\Files\IRootFolder; +use OCP\Files\Storage\IStorage; +use OCP\IServerContainer; +use OCP\IUser; +use OCP\IUserSession; +use OCP\Share\IAttributes; +use OCP\Share\IShare; +use Symfony\Component\EventDispatcher\EventDispatcher as SymfonyDispatcher; +use Test\TestCase; + +class ApplicationTest extends TestCase { + private Application $application; + private IEventDispatcher $eventDispatcher; + + /** @var IUserSession */ + private $userSession; + + /** @var IRootFolder */ + private $rootFolder; + + /** @var Manager */ private $manager; + + protected function setUp(): void { + parent::setUp(); + + $this->application = new Application([]); + + $symfonyDispatcher = new SymfonyDispatcher(); + $this->eventDispatcher = new EventDispatcher( + $symfonyDispatcher, + $this->createMock(IServerContainer::class), + $this->createMock(LoggerInterface::class) + ); + $this->userSession = $this->createMock(IUserSession::class); + $this->rootFolder = $this->createMock(IRootFolder::class); + + $this->application->registerDownloadEvents( + $this->eventDispatcher, + $this->userSession, + $this->rootFolder + ); + } + + public function providesDataForCanGet(): array { + // normal file (sender) - can download directly + $senderFileStorage = $this->createMock(IStorage::class); + $senderFileStorage->method('instanceOfStorage')->with(SharedStorage::class)->willReturn(false); + $senderFile = $this->createMock(File::class); + $senderFile->method('getStorage')->willReturn($senderFileStorage); + $senderUserFolder = $this->createMock(Folder::class); + $senderUserFolder->method('get')->willReturn($senderFile); + + $result[] = [ '/bar.txt', $senderUserFolder, true ]; + + // shared file (receiver) with attribute secure-view-enabled set false - + // can download directly + $receiverFileShareAttributes = $this->createMock(IAttributes::class); + $receiverFileShareAttributes->method('getAttribute')->with('permissions', 'download')->willReturn(true); + $receiverFileShare = $this->createMock(IShare::class); + $receiverFileShare->method('getAttributes')->willReturn($receiverFileShareAttributes); + $receiverFileStorage = $this->createMock(SharedStorage::class); + $receiverFileStorage->method('instanceOfStorage')->with(SharedStorage::class)->willReturn(true); + $receiverFileStorage->method('getShare')->willReturn($receiverFileShare); + $receiverFile = $this->createMock(File::class); + $receiverFile->method('getStorage')->willReturn($receiverFileStorage); + $receiverUserFolder = $this->createMock(Folder::class); + $receiverUserFolder->method('get')->willReturn($receiverFile); + + $result[] = [ '/share-bar.txt', $receiverUserFolder, true ]; + + // shared file (receiver) with attribute secure-view-enabled set true - + // cannot download directly + $secureReceiverFileShareAttributes = $this->createMock(IAttributes::class); + $secureReceiverFileShareAttributes->method('getAttribute')->with('permissions', 'download')->willReturn(false); + $secureReceiverFileShare = $this->createMock(IShare::class); + $secureReceiverFileShare->method('getAttributes')->willReturn($secureReceiverFileShareAttributes); + $secureReceiverFileStorage = $this->createMock(SharedStorage::class); + $secureReceiverFileStorage->method('instanceOfStorage')->with(SharedStorage::class)->willReturn(true); + $secureReceiverFileStorage->method('getShare')->willReturn($secureReceiverFileShare); + $secureReceiverFile = $this->createMock(File::class); + $secureReceiverFile->method('getStorage')->willReturn($secureReceiverFileStorage); + $secureReceiverUserFolder = $this->createMock(Folder::class); + $secureReceiverUserFolder->method('get')->willReturn($secureReceiverFile); + + $result[] = [ '/secure-share-bar.txt', $secureReceiverUserFolder, false ]; + + return $result; + } + + /** + * @dataProvider providesDataForCanGet + */ + public function testCheckDirectCanBeDownloaded(string $path, Folder $userFolder, bool $run): void { + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('test'); + $this->userSession->method('getUser')->willReturn($user); + $this->userSession->method('isLoggedIn')->willReturn(true); + $this->rootFolder->method('getUserFolder')->willReturn($userFolder); + + // Simulate direct download of file + $event = new BeforeDirectFileDownloadEvent($path); + $this->eventDispatcher->dispatchTyped($event); + + $this->assertEquals($run, $event->isSuccessful()); + } + + public function providesDataForCanZip(): array { + // Mock: Normal file/folder storage + $nonSharedStorage = $this->createMock(IStorage::class); + $nonSharedStorage->method('instanceOfStorage')->with(SharedStorage::class)->willReturn(false); + + // Mock: Secure-view file/folder shared storage + $secureReceiverFileShareAttributes = $this->createMock(IAttributes::class); + $secureReceiverFileShareAttributes->method('getAttribute')->with('permissions', 'download')->willReturn(false); + $secureReceiverFileShare = $this->createMock(IShare::class); + $secureReceiverFileShare->method('getAttributes')->willReturn($secureReceiverFileShareAttributes); + $secureSharedStorage = $this->createMock(SharedStorage::class); + $secureSharedStorage->method('instanceOfStorage')->with(SharedStorage::class)->willReturn(true); + $secureSharedStorage->method('getShare')->willReturn($secureReceiverFileShare); + + // 1. can download zipped 2 non-shared files inside non-shared folder + // 2. can download zipped non-shared folder + $sender1File = $this->createMock(File::class); + $sender1File->method('getStorage')->willReturn($nonSharedStorage); + $sender1Folder = $this->createMock(Folder::class); + $sender1Folder->method('getStorage')->willReturn($nonSharedStorage); + $sender1Folder->method('getDirectoryListing')->willReturn([$sender1File, $sender1File]); + $sender1RootFolder = $this->createMock(Folder::class); + $sender1RootFolder->method('getStorage')->willReturn($nonSharedStorage); + $sender1RootFolder->method('getDirectoryListing')->willReturn([$sender1Folder]); + $sender1UserFolder = $this->createMock(Folder::class); + $sender1UserFolder->method('get')->willReturn($sender1RootFolder); + + $return[] = [ '/folder', ['bar1.txt', 'bar2.txt'], $sender1UserFolder, true ]; + $return[] = [ '/', ['folder'], $sender1UserFolder, true ]; + + // 3. cannot download zipped 1 non-shared file and 1 secure-shared inside non-shared folder + $receiver1File = $this->createMock(File::class); + $receiver1File->method('getStorage')->willReturn($nonSharedStorage); + $receiver1SecureFile = $this->createMock(File::class); + $receiver1SecureFile->method('getStorage')->willReturn($secureSharedStorage); + $receiver1Folder = $this->createMock(Folder::class); + $receiver1Folder->method('getStorage')->willReturn($nonSharedStorage); + $receiver1Folder->method('getDirectoryListing')->willReturn([$receiver1File, $receiver1SecureFile]); + $receiver1RootFolder = $this->createMock(Folder::class); + $receiver1RootFolder->method('getStorage')->willReturn($nonSharedStorage); + $receiver1RootFolder->method('getDirectoryListing')->willReturn([$receiver1Folder]); + $receiver1UserFolder = $this->createMock(Folder::class); + $receiver1UserFolder->method('get')->willReturn($receiver1RootFolder); + + $return[] = [ '/folder', ['secured-bar1.txt', 'bar2.txt'], $receiver1UserFolder, false ]; + + // 4. cannot download zipped secure-shared folder + $receiver2Folder = $this->createMock(Folder::class); + $receiver2Folder->method('getStorage')->willReturn($secureSharedStorage); + $receiver2RootFolder = $this->createMock(Folder::class); + $receiver2RootFolder->method('getStorage')->willReturn($nonSharedStorage); + $receiver2RootFolder->method('getDirectoryListing')->willReturn([$receiver2Folder]); + $receiver2UserFolder = $this->createMock(Folder::class); + $receiver2UserFolder->method('get')->willReturn($receiver2RootFolder); + + $return[] = [ '/', ['secured-folder'], $receiver2UserFolder, false ]; + + return $return; + } + + /** + * @dataProvider providesDataForCanZip + */ + public function testCheckZipCanBeDownloaded(string $dir, array $files, Folder $userFolder, bool $run): void { + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('test'); + $this->userSession->method('getUser')->willReturn($user); + $this->userSession->method('isLoggedIn')->willReturn(true); + + $this->rootFolder->method('getUserFolder')->with('test')->willReturn($userFolder); + + // Simulate zip download of folder folder + $event = new BeforeZipCreatedEvent($dir, $files); + $this->eventDispatcher->dispatchTyped($event); + + $this->assertEquals($run, $event->isSuccessful()); + $this->assertEquals($run, $event->getErrorMessage() === null); + } + + public function testCheckFileUserNotFound(): void { + $this->userSession->method('isLoggedIn')->willReturn(false); + + // Simulate zip download of folder folder + $event = new BeforeZipCreatedEvent('/test', ['test.txt']); + $this->eventDispatcher->dispatchTyped($event); + + // It should run as this would restrict e.g. share links otherwise + $this->assertTrue($event->isSuccessful()); + $this->assertEquals(null, $event->getErrorMessage()); + } +} diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php index 5555aef1425..8665d649aad 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -46,6 +46,7 @@ use OCP\Files\IRootFolder; use OCP\Files\Mount\IMountPoint; use OCP\Files\NotFoundException; use OCP\Files\Storage; +use OCP\Files\Storage\IStorage; use OCP\IConfig; use OCP\IGroup; use OCP\IGroupManager; @@ -58,6 +59,7 @@ use OCP\IUser; use OCP\IUserManager; use OCP\Lock\LockedException; use OCP\Share\Exceptions\GenericShareException; +use OCP\Share\IAttributes as IShareAttributes; use OCP\Share\IManager; use OCP\Share\IShare; use Test\TestCase; @@ -124,7 +126,7 @@ class ShareAPIControllerTest extends TestCase { ->willReturn(true); $this->shareManager ->expects($this->any()) - ->method('shareProviderExists')->willReturn(true); + ->method('shareProviderExists')->willReturn(true); $this->groupManager = $this->createMock(IGroupManager::class); $this->userManager = $this->createMock(IUserManager::class); $this->request = $this->createMock(IRequest::class); @@ -194,6 +196,23 @@ class ShareAPIControllerTest extends TestCase { } + private function mockShareAttributes() { + $formattedShareAttributes = [ + [ + 'scope' => 'permissions', + 'key' => 'download', + 'enabled' => true + ] + ]; + + $shareAttributes = $this->createMock(IShareAttributes::class); + $shareAttributes->method('toArray')->willReturn($formattedShareAttributes); + $shareAttributes->method('getAttribute')->with('permissions', 'download')->willReturn(true); + + // send both IShare attributes class and expected json string + return [$shareAttributes, \json_encode($formattedShareAttributes)]; + } + public function testDeleteShareShareNotFound() { $this->expectException(\OCP\AppFramework\OCS\OCSNotFoundException::class); $this->expectExceptionMessage('Wrong share ID, share doesn\'t exist'); @@ -505,7 +524,7 @@ class ShareAPIControllerTest extends TestCase { public function createShare($id, $shareType, $sharedWith, $sharedBy, $shareOwner, $path, $permissions, $shareTime, $expiration, $parent, $target, $mail_send, $note = '', $token = null, - $password = null, $label = '') { + $password = null, $label = '', $attributes = null) { $share = $this->getMockBuilder(IShare::class)->getMock(); $share->method('getId')->willReturn($id); $share->method('getShareType')->willReturn($shareType); @@ -516,6 +535,7 @@ class ShareAPIControllerTest extends TestCase { $share->method('getPermissions')->willReturn($permissions); $share->method('getNote')->willReturn($note); $share->method('getLabel')->willReturn($label); + $share->method('getAttributes')->willReturn($attributes); $time = new \DateTime(); $time->setTimestamp($shareTime); $share->method('getShareTime')->willReturn($time); @@ -565,6 +585,8 @@ class ShareAPIControllerTest extends TestCase { $folder->method('getParent')->willReturn($parentFolder); $folder->method('getMimeType')->willReturn('myFolderMimeType'); + [$shareAttributes, $shareAttributesReturnJson] = $this->mockShareAttributes(); + // File shared with user $share = $this->createShare( 100, @@ -579,7 +601,8 @@ class ShareAPIControllerTest extends TestCase { 6, 'target', 0, - 'personal note' + 'personal note', + $shareAttributes, ); $expected = [ 'id' => 100, @@ -597,6 +620,7 @@ class ShareAPIControllerTest extends TestCase { 'token' => null, 'expiration' => null, 'permissions' => 4, + 'attributes' => $shareAttributesReturnJson, 'stime' => 5, 'parent' => null, 'storage_id' => 'STORAGE', @@ -613,6 +637,7 @@ class ShareAPIControllerTest extends TestCase { 'can_edit' => false, 'can_delete' => false, 'status' => [], + 'attributes' => null, ]; $data[] = [$share, $expected]; @@ -630,7 +655,8 @@ class ShareAPIControllerTest extends TestCase { 6, 'target', 0, - 'personal note' + 'personal note', + $shareAttributes, ); $expected = [ 'id' => 101, @@ -647,6 +673,7 @@ class ShareAPIControllerTest extends TestCase { 'token' => null, 'expiration' => null, 'permissions' => 4, + 'attributes' => $shareAttributesReturnJson, 'stime' => 5, 'parent' => null, 'storage_id' => 'STORAGE', @@ -662,6 +689,7 @@ class ShareAPIControllerTest extends TestCase { 'hide_download' => 0, 'can_edit' => false, 'can_delete' => false, + 'attributes' => null, ]; $data[] = [$share, $expected]; @@ -702,6 +730,7 @@ class ShareAPIControllerTest extends TestCase { 'token' => 'token', 'expiration' => '2000-01-02 00:00:00', 'permissions' => 4, + 'attributes' => null, 'stime' => 5, 'parent' => null, 'storage_id' => 'STORAGE', @@ -718,6 +747,7 @@ class ShareAPIControllerTest extends TestCase { 'hide_download' => 0, 'can_edit' => false, 'can_delete' => false, + 'attributes' => null, ]; $data[] = [$share, $expected]; @@ -1646,8 +1676,10 @@ class ShareAPIControllerTest extends TestCase { $path = $this->getMockBuilder(File::class)->getMock(); $storage = $this->createMock(Storage::class); $storage->method('instanceOfStorage') - ->with('OCA\Files_Sharing\External\Storage') - ->willReturn(false); + ->willReturnMap([ + ['OCA\Files_Sharing\External\Storage', false], + ['OCA\Files_Sharing\SharedStorage', false], + ]); $path->method('getStorage')->willReturn($storage); $userFolder->expects($this->once()) ->method('get') @@ -1680,8 +1712,10 @@ class ShareAPIControllerTest extends TestCase { $path = $this->getMockBuilder(File::class)->getMock(); $storage = $this->createMock(Storage::class); $storage->method('instanceOfStorage') - ->with('OCA\Files_Sharing\External\Storage') - ->willReturn(false); + ->willReturnMap([ + ['OCA\Files_Sharing\External\Storage', false], + ['OCA\Files_Sharing\SharedStorage', false], + ]); $path->method('getStorage')->willReturn($storage); $userFolder->expects($this->once()) ->method('get') @@ -1732,8 +1766,10 @@ class ShareAPIControllerTest extends TestCase { $path = $this->getMockBuilder(File::class)->getMock(); $storage = $this->createMock(Storage::class); $storage->method('instanceOfStorage') - ->with('OCA\Files_Sharing\External\Storage') - ->willReturn(false); + ->willReturnMap([ + ['OCA\Files_Sharing\External\Storage', false], + ['OCA\Files_Sharing\SharedStorage', false], + ]); $path->method('getStorage')->willReturn($storage); $userFolder->expects($this->once()) ->method('get') @@ -1788,8 +1824,10 @@ class ShareAPIControllerTest extends TestCase { $path = $this->getMockBuilder(File::class)->getMock(); $storage = $this->createMock(Storage::class); $storage->method('instanceOfStorage') - ->with('OCA\Files_Sharing\External\Storage') - ->willReturn(false); + ->willReturnMap([ + ['OCA\Files_Sharing\External\Storage', false], + ['OCA\Files_Sharing\SharedStorage', false], + ]); $path->method('getStorage')->willReturn($storage); $userFolder->expects($this->once()) ->method('get') @@ -1847,8 +1885,10 @@ class ShareAPIControllerTest extends TestCase { $path = $this->getMockBuilder(Folder::class)->getMock(); $storage = $this->createMock(Storage::class); $storage->method('instanceOfStorage') - ->with('OCA\Files_Sharing\External\Storage') - ->willReturn(false); + ->willReturnMap([ + ['OCA\Files_Sharing\External\Storage', false], + ['OCA\Files_Sharing\SharedStorage', false], + ]); $path->method('getStorage')->willReturn($storage); $userFolder->expects($this->once()) ->method('get') @@ -1901,8 +1941,10 @@ class ShareAPIControllerTest extends TestCase { $path = $this->getMockBuilder(Folder::class)->getMock(); $storage = $this->createMock(Storage::class); $storage->method('instanceOfStorage') - ->with('OCA\Files_Sharing\External\Storage') - ->willReturn(false); + ->willReturnMap([ + ['OCA\Files_Sharing\External\Storage', false], + ['OCA\Files_Sharing\SharedStorage', false], + ]); $path->method('getStorage')->willReturn($storage); $userFolder->expects($this->once()) ->method('get') @@ -1935,8 +1977,10 @@ class ShareAPIControllerTest extends TestCase { $path = $this->getMockBuilder(Folder::class)->getMock(); $storage = $this->createMock(Storage::class); $storage->method('instanceOfStorage') - ->with('OCA\Files_Sharing\External\Storage') - ->willReturn(false); + ->willReturnMap([ + ['OCA\Files_Sharing\External\Storage', false], + ['OCA\Files_Sharing\SharedStorage', false], + ]); $path->method('getStorage')->willReturn($storage); $this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf(); $this->rootFolder->method('get')->with('valid-path')->willReturn($path); @@ -1956,8 +2000,10 @@ class ShareAPIControllerTest extends TestCase { $path = $this->getMockBuilder(Folder::class)->getMock(); $storage = $this->createMock(Storage::class); $storage->method('instanceOfStorage') - ->with('OCA\Files_Sharing\External\Storage') - ->willReturn(false); + ->willReturnMap([ + ['OCA\Files_Sharing\External\Storage', false], + ['OCA\Files_Sharing\SharedStorage', false], + ]); $path->method('getStorage')->willReturn($storage); $this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf(); $this->rootFolder->method('get')->with('valid-path')->willReturn($path); @@ -1978,8 +2024,10 @@ class ShareAPIControllerTest extends TestCase { $path = $this->getMockBuilder(File::class)->getMock(); $storage = $this->createMock(Storage::class); $storage->method('instanceOfStorage') - ->with('OCA\Files_Sharing\External\Storage') - ->willReturn(false); + ->willReturnMap([ + ['OCA\Files_Sharing\External\Storage', false], + ['OCA\Files_Sharing\SharedStorage', false], + ]); $path->method('getStorage')->willReturn($storage); $this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf(); $this->rootFolder->method('get')->with('valid-path')->willReturn($path); @@ -1999,8 +2047,10 @@ class ShareAPIControllerTest extends TestCase { $path = $this->getMockBuilder(Folder::class)->getMock(); $storage = $this->createMock(Storage::class); $storage->method('instanceOfStorage') - ->with('OCA\Files_Sharing\External\Storage') - ->willReturn(false); + ->willReturnMap([ + ['OCA\Files_Sharing\External\Storage', false], + ['OCA\Files_Sharing\SharedStorage', false], + ]); $path->method('getStorage')->willReturn($storage); $this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf(); $this->rootFolder->method('get')->with('valid-path')->willReturn($path); @@ -2035,8 +2085,10 @@ class ShareAPIControllerTest extends TestCase { $path = $this->getMockBuilder(Folder::class)->getMock(); $storage = $this->createMock(Storage::class); $storage->method('instanceOfStorage') - ->with('OCA\Files_Sharing\External\Storage') - ->willReturn(false); + ->willReturnMap([ + ['OCA\Files_Sharing\External\Storage', false], + ['OCA\Files_Sharing\SharedStorage', false], + ]); $path->method('getStorage')->willReturn($storage); $this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf(); $this->rootFolder->method('get')->with('valid-path')->willReturn($path); @@ -2071,8 +2123,10 @@ class ShareAPIControllerTest extends TestCase { $path = $this->getMockBuilder(Folder::class)->getMock(); $storage = $this->createMock(Storage::class); $storage->method('instanceOfStorage') - ->with('OCA\Files_Sharing\External\Storage') - ->willReturn(false); + ->willReturnMap([ + ['OCA\Files_Sharing\External\Storage', false], + ['OCA\Files_Sharing\SharedStorage', false], + ]); $path->method('getStorage')->willReturn($storage); $this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf(); $this->rootFolder->method('get')->with('valid-path')->willReturn($path); @@ -2114,8 +2168,10 @@ class ShareAPIControllerTest extends TestCase { $path = $this->getMockBuilder(Folder::class)->getMock(); $storage = $this->createMock(Storage::class); $storage->method('instanceOfStorage') - ->with('OCA\Files_Sharing\External\Storage') - ->willReturn(false); + ->willReturnMap([ + ['OCA\Files_Sharing\External\Storage', false], + ['OCA\Files_Sharing\SharedStorage', false], + ]); $path->method('getStorage')->willReturn($storage); $path->method('getPath')->willReturn('valid-path'); $this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf(); @@ -2150,8 +2206,10 @@ class ShareAPIControllerTest extends TestCase { $path = $this->getMockBuilder(Folder::class)->getMock(); $storage = $this->createMock(Storage::class); $storage->method('instanceOfStorage') - ->with('OCA\Files_Sharing\External\Storage') - ->willReturn(false); + ->willReturnMap([ + ['OCA\Files_Sharing\External\Storage', false], + ['OCA\Files_Sharing\SharedStorage', false], + ]); $path->method('getStorage')->willReturn($storage); $this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf(); $this->rootFolder->method('get')->with('valid-path')->willReturn($path); @@ -2193,8 +2251,10 @@ class ShareAPIControllerTest extends TestCase { $path = $this->getMockBuilder(Folder::class)->getMock(); $storage = $this->createMock(Storage::class); $storage->method('instanceOfStorage') - ->with('OCA\Files_Sharing\External\Storage') - ->willReturn(false); + ->willReturnMap([ + ['OCA\Files_Sharing\External\Storage', false], + ['OCA\Files_Sharing\SharedStorage', false], + ]); $path->method('getStorage')->willReturn($storage); $this->rootFolder->method('getUserFolder')->with($this->currentUser)->willReturnSelf(); $this->rootFolder->method('get')->with('valid-path')->willReturn($path); @@ -2241,8 +2301,10 @@ class ShareAPIControllerTest extends TestCase { $path = $this->getMockBuilder(File::class)->getMock(); $storage = $this->createMock(Storage::class); $storage->method('instanceOfStorage') - ->with('OCA\Files_Sharing\External\Storage') - ->willReturn(false); + ->willReturnMap([ + ['OCA\Files_Sharing\External\Storage', false], + ['OCA\Files_Sharing\SharedStorage', false], + ]); $path->method('getStorage')->willReturn($storage); $userFolder->expects($this->once()) ->method('get') @@ -2313,8 +2375,10 @@ class ShareAPIControllerTest extends TestCase { $path = $this->getMockBuilder(File::class)->getMock(); $storage = $this->createMock(Storage::class); $storage->method('instanceOfStorage') - ->with('OCA\Files_Sharing\External\Storage') - ->willReturn(false); + ->willReturnMap([ + ['OCA\Files_Sharing\External\Storage', false], + ['OCA\Files_Sharing\SharedStorage', false], + ]); $path->method('getStorage')->willReturn($storage); $userFolder->expects($this->once()) ->method('get') @@ -2367,8 +2431,10 @@ class ShareAPIControllerTest extends TestCase { $path = $this->getMockBuilder(File::class)->getMock(); $storage = $this->createMock(Storage::class); $storage->method('instanceOfStorage') - ->with('OCA\Files_Sharing\External\Storage') - ->willReturn(false); + ->willReturnMap([ + ['OCA\Files_Sharing\External\Storage', false], + ['OCA\Files_Sharing\SharedStorage', false], + ]); $path->method('getStorage')->willReturn($storage); $userFolder->expects($this->once()) ->method('get') @@ -2451,8 +2517,10 @@ class ShareAPIControllerTest extends TestCase { $path = $this->getMockBuilder(File::class)->getMock(); $storage = $this->createMock(Storage::class); $storage->method('instanceOfStorage') - ->with('OCA\Files_Sharing\External\Storage') - ->willReturn(false); + ->willReturnMap([ + ['OCA\Files_Sharing\External\Storage', false], + ['OCA\Files_Sharing\SharedStorage', false], + ]); $path->method('getStorage')->willReturn($storage); $path->method('getPath')->willReturn('valid-path'); $userFolder->expects($this->once()) @@ -2494,8 +2562,10 @@ class ShareAPIControllerTest extends TestCase { $path = $this->getMockBuilder(File::class)->getMock(); $storage = $this->createMock(Storage::class); $storage->method('instanceOfStorage') - ->with('OCA\Files_Sharing\External\Storage') - ->willReturn(false); + ->willReturnMap([ + ['OCA\Files_Sharing\External\Storage', false], + ['OCA\Files_Sharing\SharedStorage', false], + ]); $path->method('getStorage')->willReturn($storage); $userFolder->expects($this->once()) ->method('get') @@ -2575,8 +2645,10 @@ class ShareAPIControllerTest extends TestCase { $path = $this->getMockBuilder(Folder::class)->getMock(); $storage = $this->createMock(Storage::class); $storage->method('instanceOfStorage') - ->with('OCA\Files_Sharing\External\Storage') - ->willReturn(true); + ->willReturnMap([ + ['OCA\Files_Sharing\External\Storage', true], + ['OCA\Files_Sharing\SharedStorage', false], + ]); $path->method('getStorage')->willReturn($storage); $path->method('getPermissions')->willReturn(\OCP\Constants::PERMISSION_READ); $userFolder->expects($this->once()) @@ -2935,8 +3007,17 @@ class ShareAPIControllerTest extends TestCase { $this->expectExceptionMessage('Invalid date. Format must be YYYY-MM-DD'); $ocs = $this->mockFormatShare(); + $userFolder = $this->createMock(Folder::class); + $userFolder->method('getById') + ->with(42) + ->willReturn([]); + $this->rootFolder->method('getUserFolder') + ->with($this->currentUser) + ->willReturn($userFolder); $folder = $this->getMockBuilder(Folder::class)->getMock(); + $folder->method('getId') + ->willReturn(42); $share = \OC::$server->getShareManager()->newShare(); $share->setPermissions(\OCP\Constants::PERMISSION_ALL) @@ -2974,8 +3055,16 @@ class ShareAPIControllerTest extends TestCase { $this->expectExceptionMessage('Public upload disabled by the administrator'); $ocs = $this->mockFormatShare(); + $userFolder = $this->createMock(Folder::class); + $userFolder->method('getById') + ->with(42) + ->willReturn([]); + $this->rootFolder->method('getUserFolder') + ->with($this->currentUser) + ->willReturn($userFolder); $folder = $this->getMockBuilder(Folder::class)->getMock(); + $folder->method('getId')->willReturn(42); $share = \OC::$server->getShareManager()->newShare(); $share->setPermissions(\OCP\Constants::PERMISSION_ALL) @@ -2997,6 +3086,15 @@ class ShareAPIControllerTest extends TestCase { $ocs = $this->mockFormatShare(); $file = $this->getMockBuilder(File::class)->getMock(); + $file->method('getId') + ->willReturn(42); + $userFolder = $this->createMock(Folder::class); + $userFolder->method('getById') + ->with(42) + ->willReturn([]); + $this->rootFolder->method('getUserFolder') + ->with($this->currentUser) + ->willReturn($userFolder); $share = \OC::$server->getShareManager()->newShare(); $share->setPermissions(\OCP\Constants::PERMISSION_ALL) @@ -3010,13 +3108,21 @@ class ShareAPIControllerTest extends TestCase { $ocs->updateShare(42, null, 'password', null, 'true', ''); } - public function testUpdateLinkSharePasswordDoesNotChangeOther() { + public function testUpdateLinkSharePasswordDoesNotChangeOther(): void { $ocs = $this->mockFormatShare(); $date = new \DateTime('2000-01-01'); $date->setTime(0,0,0); $node = $this->getMockBuilder(File::class)->getMock(); + $node->method('getId')->willReturn(42); + $userFolder = $this->createMock(Folder::class); + $userFolder->method('getById') + ->with(42) + ->willReturn([]); + $this->rootFolder->method('getUserFolder') + ->with($this->currentUser) + ->willReturn($userFolder); $share = $this->newShare(); $share->setPermissions(\OCP\Constants::PERMISSION_ALL) ->setSharedBy($this->currentUser) @@ -3061,7 +3167,15 @@ class ShareAPIControllerTest extends TestCase { $date = new \DateTime('2000-01-01'); $date->setTime(0,0,0); + $userFolder = $this->createMock(Folder::class); + $userFolder->method('getById') + ->with(42) + ->willReturn([]); + $this->rootFolder->method('getUserFolder') + ->with($this->currentUser) + ->willReturn($userFolder); $node = $this->getMockBuilder(File::class)->getMock(); + $node->method('getId')->willReturn(42); $share = $this->newShare(); $share->setPermissions(\OCP\Constants::PERMISSION_ALL) ->setSharedBy($this->currentUser) @@ -3112,7 +3226,15 @@ class ShareAPIControllerTest extends TestCase { $date = new \DateTime('2000-01-01'); $date->setTime(0,0,0); + $userFolder = $this->createMock(Folder::class); + $userFolder->method('getById') + ->with(42) + ->willReturn([]); + $this->rootFolder->method('getUserFolder') + ->with($this->currentUser) + ->willReturn($userFolder); $node = $this->getMockBuilder(File::class)->getMock(); + $node->method('getId')->willReturn(42); $share = $this->newShare(); $share->setPermissions(\OCP\Constants::PERMISSION_ALL) ->setSharedBy($this->currentUser) @@ -3145,7 +3267,15 @@ class ShareAPIControllerTest extends TestCase { $date = new \DateTime('2000-01-01'); $date->setTime(0,0,0); + $userFolder = $this->createMock(Folder::class); + $userFolder->method('getById') + ->with(42) + ->willReturn([]); + $this->rootFolder->method('getUserFolder') + ->with($this->currentUser) + ->willReturn($userFolder); $node = $this->getMockBuilder(File::class)->getMock(); + $node->method('getId')->willReturn(42); $share = $this->newShare(); $share->setPermissions(\OCP\Constants::PERMISSION_ALL) ->setSharedBy($this->currentUser) @@ -3725,7 +3855,7 @@ class ShareAPIControllerTest extends TestCase { $recipient = $this->getMockBuilder(IUser::class)->getMock(); $recipient->method('getDisplayName')->willReturn('recipientDN'); $recipient->method('getSystemEMailAddress')->willReturn('recipient'); - + [$shareAttributes, $shareAttributesReturnJson] = $this->mockShareAttributes(); $result = []; @@ -3735,6 +3865,7 @@ class ShareAPIControllerTest extends TestCase { ->setSharedBy('initiator') ->setShareOwner('owner') ->setPermissions(\OCP\Constants::PERMISSION_READ) + ->setAttributes($shareAttributes) ->setNode($file) ->setShareTime(new \DateTime('2000-01-01T00:01:02')) ->setTarget('myTarget') @@ -3749,6 +3880,7 @@ class ShareAPIControllerTest extends TestCase { 'uid_owner' => 'initiator', 'displayname_owner' => 'initiator', 'permissions' => 1, + 'attributes' => $shareAttributesReturnJson, 'stime' => 946684862, 'parent' => null, 'expiration' => null, @@ -3775,6 +3907,7 @@ class ShareAPIControllerTest extends TestCase { 'can_edit' => false, 'can_delete' => false, 'status' => [], + 'attributes' => '[{"scope":"permissions","key":"download","enabled":true}]', ], $share, [], false ]; // User backend up @@ -3785,6 +3918,7 @@ class ShareAPIControllerTest extends TestCase { 'uid_owner' => 'initiator', 'displayname_owner' => 'initiatorDN', 'permissions' => 1, + 'attributes' => $shareAttributesReturnJson, 'stime' => 946684862, 'parent' => null, 'expiration' => null, @@ -3811,6 +3945,7 @@ class ShareAPIControllerTest extends TestCase { 'can_edit' => false, 'can_delete' => false, 'status' => [], + 'attributes' => '[{"scope":"permissions","key":"download","enabled":true}]', ], $share, [ ['owner', $owner], ['initiator', $initiator], @@ -3837,6 +3972,7 @@ class ShareAPIControllerTest extends TestCase { 'uid_owner' => 'initiator', 'displayname_owner' => 'initiator', 'permissions' => 1, + 'attributes' => null, 'stime' => 946684862, 'parent' => null, 'expiration' => null, @@ -3863,6 +3999,7 @@ class ShareAPIControllerTest extends TestCase { 'can_edit' => false, 'can_delete' => false, 'status' => [], + 'attributes' => null, ], $share, [], false ]; @@ -3885,6 +4022,7 @@ class ShareAPIControllerTest extends TestCase { 'uid_owner' => 'initiator', 'displayname_owner' => 'initiator', 'permissions' => 1, + 'attributes' => null, 'stime' => 946684862, 'parent' => null, 'expiration' => null, @@ -3911,6 +4049,7 @@ class ShareAPIControllerTest extends TestCase { 'can_edit' => true, 'can_delete' => true, 'status' => [], + 'attributes' => null, ], $share, [], false ]; @@ -3935,6 +4074,7 @@ class ShareAPIControllerTest extends TestCase { 'uid_owner' => 'initiator', 'displayname_owner' => 'initiator', 'permissions' => 1, + 'attributes' => null, 'stime' => 946684862, 'parent' => null, 'expiration' => null, @@ -3959,6 +4099,7 @@ class ShareAPIControllerTest extends TestCase { 'hide_download' => 0, 'can_edit' => false, 'can_delete' => false, + 'attributes' => null, ], $share, [], false ]; @@ -4005,6 +4146,7 @@ class ShareAPIControllerTest extends TestCase { 'hide_download' => 0, 'can_edit' => false, 'can_delete' => false, + 'attributes' => null, ], $share, [], false ]; @@ -4030,6 +4172,7 @@ class ShareAPIControllerTest extends TestCase { 'uid_owner' => 'initiator', 'displayname_owner' => 'initiator', 'permissions' => 1, + 'attributes' => null, 'stime' => 946684862, 'parent' => null, 'expiration' => '2001-01-02 00:00:00', @@ -4057,6 +4200,7 @@ class ShareAPIControllerTest extends TestCase { 'hide_download' => 0, 'can_edit' => false, 'can_delete' => false, + 'attributes' => null, ], $share, [], false ]; @@ -4110,6 +4254,7 @@ class ShareAPIControllerTest extends TestCase { 'hide_download' => 0, 'can_edit' => false, 'can_delete' => false, + 'attributes' => null, ], $share, [], false ]; @@ -4157,6 +4302,7 @@ class ShareAPIControllerTest extends TestCase { 'hide_download' => 0, 'can_edit' => false, 'can_delete' => false, + 'attributes' => null, ], $share, [], false ]; @@ -4204,6 +4350,7 @@ class ShareAPIControllerTest extends TestCase { 'hide_download' => 0, 'can_edit' => false, 'can_delete' => false, + 'attributes' => null, ], $share, [], false ]; @@ -4228,6 +4375,7 @@ class ShareAPIControllerTest extends TestCase { 'uid_owner' => 'initiator', 'displayname_owner' => 'initiator', 'permissions' => 1, + 'attributes' => null, 'stime' => 946684862, 'parent' => null, 'expiration' => null, @@ -4253,6 +4401,7 @@ class ShareAPIControllerTest extends TestCase { 'hide_download' => 0, 'can_edit' => false, 'can_delete' => false, + 'attributes' => null, ], $share, [], false ]; @@ -4300,6 +4449,7 @@ class ShareAPIControllerTest extends TestCase { 'hide_download' => 0, 'can_edit' => false, 'can_delete' => false, + 'attributes' => null, ], $share, [], false ]; @@ -4347,6 +4497,7 @@ class ShareAPIControllerTest extends TestCase { 'hide_download' => 0, 'can_edit' => false, 'can_delete' => false, + 'attributes' => null, ], $share, [], false ]; @@ -4411,6 +4562,7 @@ class ShareAPIControllerTest extends TestCase { 'can_edit' => false, 'can_delete' => false, 'password_expiration_time' => null, + 'attributes' => null, ], $share, [], false ]; @@ -4461,6 +4613,7 @@ class ShareAPIControllerTest extends TestCase { 'can_edit' => false, 'can_delete' => false, 'password_expiration_time' => null, + 'attributes' => null, ], $share, [], false ]; @@ -4510,6 +4663,7 @@ class ShareAPIControllerTest extends TestCase { 'can_edit' => true, 'can_delete' => true, 'status' => [], + 'attributes' => null, ], $share, [], false ]; @@ -4661,6 +4815,7 @@ class ShareAPIControllerTest extends TestCase { 'label' => '', 'can_edit' => false, 'can_delete' => false, + 'attributes' => null, ], $share, false, [] ]; @@ -4707,6 +4862,7 @@ class ShareAPIControllerTest extends TestCase { 'label' => '', 'can_edit' => false, 'can_delete' => false, + 'attributes' => null, ], $share, true, [ 'share_with_displayname' => 'recipientRoomName' ] diff --git a/apps/files_sharing/tests/External/ScannerTest.php b/apps/files_sharing/tests/External/ScannerTest.php index 57696a697eb..2d2486737dc 100644 --- a/apps/files_sharing/tests/External/ScannerTest.php +++ b/apps/files_sharing/tests/External/ScannerTest.php @@ -50,18 +50,6 @@ class ScannerTest extends TestCase { $this->scanner = new Scanner($this->storage); } - public function testScanAll() { - $this->storage->expects($this->any()) - ->method('getShareInfo') - ->willReturn(['status' => 'success', 'data' => []]); - - // FIXME add real tests, we are currently only checking for - // Declaration of OCA\Files_Sharing\External\Scanner::*() should be - // compatible with OC\Files\Cache\Scanner::*() - $this->scanner->scanAll(); - $this->addToAssertionCount(1); - } - public function testScan() { $this->storage->expects($this->any()) ->method('getShareInfo') diff --git a/apps/files_sharing/tests/MountProviderTest.php b/apps/files_sharing/tests/MountProviderTest.php index 00ae847eaac..741f9b7128a 100644 --- a/apps/files_sharing/tests/MountProviderTest.php +++ b/apps/files_sharing/tests/MountProviderTest.php @@ -39,6 +39,7 @@ use OCP\IConfig; use OCP\ILogger; use OCP\IUser; use OCP\IUserManager; +use OCP\Share\IAttributes as IShareAttributes; use OCP\Share\IManager; use OCP\Share\IShare; @@ -81,12 +82,36 @@ class MountProviderTest extends \Test\TestCase { $this->provider = new MountProvider($this->config, $this->shareManager, $this->logger, $eventDispatcher, $cacheFactory); } - private function makeMockShare($id, $nodeId, $owner = 'user2', $target = null, $permissions = 31) { + private function makeMockShareAttributes($attrs) { + if ($attrs === null) { + return null; + } + + $shareAttributes = $this->createMock(IShareAttributes::class); + $shareAttributes->method('toArray')->willReturn($attrs); + $shareAttributes->method('getAttribute')->will( + $this->returnCallback(function ($scope, $key) use ($attrs) { + $result = null; + foreach ($attrs as $attr) { + if ($attr['key'] === $key && $attr['scope'] === $scope) { + $result = $attr['enabled']; + } + } + return $result; + }) + ); + return $shareAttributes; + } + + private function makeMockShare($id, $nodeId, $owner = 'user2', $target = null, $permissions = 31, $attributes = null) { $share = $this->createMock(IShare::class); $share->expects($this->any()) ->method('getPermissions') ->willReturn($permissions); $share->expects($this->any()) + ->method('getAttributes') + ->will($this->returnValue($this->makeMockShareAttributes($attributes))); + $share->expects($this->any()) ->method('getShareOwner') ->willReturn($owner); $share->expects($this->any()) @@ -115,14 +140,16 @@ class MountProviderTest extends \Test\TestCase { public function testExcludeShares() { $rootFolder = $this->createMock(IRootFolder::class); $userManager = $this->createMock(IUserManager::class); + $attr1 = []; + $attr2 = [['scope' => 'permission', 'key' => 'download', 'enabled' => true]]; $userShares = [ - $this->makeMockShare(1, 100, 'user2', '/share2', 0), - $this->makeMockShare(2, 100, 'user2', '/share2', 31), + $this->makeMockShare(1, 100, 'user2', '/share2', 0, $attr1), + $this->makeMockShare(2, 100, 'user2', '/share2', 31, $attr2), ]; $groupShares = [ - $this->makeMockShare(3, 100, 'user2', '/share2', 0), - $this->makeMockShare(4, 101, 'user2', '/share4', 31), - $this->makeMockShare(5, 100, 'user1', '/share4', 31), + $this->makeMockShare(3, 100, 'user2', '/share2', 0, $attr1), + $this->makeMockShare(4, 101, 'user2', '/share4', 31, $attr2), + $this->makeMockShare(5, 100, 'user1', '/share4', 31, $attr2), ]; $roomShares = [ $this->makeMockShare(6, 102, 'user2', '/share6', 0), @@ -178,12 +205,14 @@ class MountProviderTest extends \Test\TestCase { $this->assertEquals(100, $mountedShare1->getNodeId()); $this->assertEquals('/share2', $mountedShare1->getTarget()); $this->assertEquals(31, $mountedShare1->getPermissions()); + $this->assertEquals(true, $mountedShare1->getAttributes()->getAttribute('permission', 'download')); $mountedShare2 = $mounts[1]->getShare(); $this->assertEquals('4', $mountedShare2->getId()); $this->assertEquals('user2', $mountedShare2->getShareOwner()); $this->assertEquals(101, $mountedShare2->getNodeId()); $this->assertEquals('/share4', $mountedShare2->getTarget()); $this->assertEquals(31, $mountedShare2->getPermissions()); + $this->assertEquals(true, $mountedShare2->getAttributes()->getAttribute('permission', 'download')); $mountedShare3 = $mounts[2]->getShare(); $this->assertEquals('8', $mountedShare3->getId()); $this->assertEquals('user2', $mountedShare3->getShareOwner()); @@ -205,27 +234,27 @@ class MountProviderTest extends \Test\TestCase { // #0: share as outsider with "group1" and "user1" with same permissions [ [ - [1, 100, 'user2', '/share2', 31], + [1, 100, 'user2', '/share2', 31, null], ], [ - [2, 100, 'user2', '/share2', 31], + [2, 100, 'user2', '/share2', 31, null], ], [ // combined, user share has higher priority - ['1', 100, 'user2', '/share2', 31], + ['1', 100, 'user2', '/share2', 31, []], ], ], // #1: share as outsider with "group1" and "user1" with different permissions [ [ - [1, 100, 'user2', '/share', 31], + [1, 100, 'user2', '/share', 31, [['scope' => 'permission', 'key' => 'download', 'enabled' => true], ['scope' => 'app', 'key' => 'attribute1', 'enabled' => true]]], ], [ - [2, 100, 'user2', '/share', 15], + [2, 100, 'user2', '/share', 15, [['scope' => 'permission', 'key' => 'download', 'enabled' => false], ['scope' => 'app', 'key' => 'attribute2', 'enabled' => false]]], ], [ // use highest permissions - ['1', 100, 'user2', '/share', 31], + ['1', 100, 'user2', '/share', 31, [['scope' => 'permission', 'key' => 'download', 'enabled' => true], ['scope' => 'app', 'key' => 'attribute1', 'enabled' => true], ['scope' => 'app', 'key' => 'attribute2', 'enabled' => false]]], ], ], // #2: share as outsider with "group1" and "group2" with same permissions @@ -233,12 +262,12 @@ class MountProviderTest extends \Test\TestCase { [ ], [ - [1, 100, 'user2', '/share', 31], - [2, 100, 'user2', '/share', 31], + [1, 100, 'user2', '/share', 31, null], + [2, 100, 'user2', '/share', 31, []], ], [ // combined, first group share has higher priority - ['1', 100, 'user2', '/share', 31], + ['1', 100, 'user2', '/share', 31, []], ], ], // #3: share as outsider with "group1" and "group2" with different permissions @@ -246,12 +275,12 @@ class MountProviderTest extends \Test\TestCase { [ ], [ - [1, 100, 'user2', '/share', 31], - [2, 100, 'user2', '/share', 15], + [1, 100, 'user2', '/share', 31, [['scope' => 'permission', 'key' => 'download', 'enabled' => false]]], + [2, 100, 'user2', '/share', 15, [['scope' => 'permission', 'key' => 'download', 'enabled' => true]]], ], [ - // use higher permissions - ['1', 100, 'user2', '/share', 31], + // use higher permissions (most permissive) + ['1', 100, 'user2', '/share', 31, [['scope' => 'permission', 'key' => 'download', 'enabled' => true]]], ], ], // #4: share as insider with "group1" @@ -259,7 +288,7 @@ class MountProviderTest extends \Test\TestCase { [ ], [ - [1, 100, 'user1', '/share', 31], + [1, 100, 'user1', '/share', 31, []], ], [ // no received share since "user1" is the sharer/owner @@ -270,8 +299,8 @@ class MountProviderTest extends \Test\TestCase { [ ], [ - [1, 100, 'user1', '/share', 31], - [2, 100, 'user1', '/share', 15], + [1, 100, 'user1', '/share', 31, [['scope' => 'permission', 'key' => 'download', 'enabled' => true]]], + [2, 100, 'user1', '/share', 15, [['scope' => 'permission', 'key' => 'download', 'enabled' => false]]], ], [ // no received share since "user1" is the sharer/owner @@ -282,7 +311,7 @@ class MountProviderTest extends \Test\TestCase { [ ], [ - [1, 100, 'user2', '/share', 0], + [1, 100, 'user2', '/share', 0, []], ], [ // no received share since "user1" opted out @@ -291,40 +320,40 @@ class MountProviderTest extends \Test\TestCase { // #7: share as outsider with "group1" and "user1" where recipient renamed in between [ [ - [1, 100, 'user2', '/share2-renamed', 31], + [1, 100, 'user2', '/share2-renamed', 31, []], ], [ - [2, 100, 'user2', '/share2', 31], + [2, 100, 'user2', '/share2', 31, []], ], [ // use target of least recent share - ['1', 100, 'user2', '/share2-renamed', 31], + ['1', 100, 'user2', '/share2-renamed', 31, []], ], ], // #8: share as outsider with "group1" and "user1" where recipient renamed in between [ [ - [2, 100, 'user2', '/share2', 31], + [2, 100, 'user2', '/share2', 31, []], ], [ - [1, 100, 'user2', '/share2-renamed', 31], + [1, 100, 'user2', '/share2-renamed', 31, []], ], [ // use target of least recent share - ['1', 100, 'user2', '/share2-renamed', 31], + ['1', 100, 'user2', '/share2-renamed', 31, []], ], ], // #9: share as outsider with "nullgroup" and "user1" where recipient renamed in between [ [ - [2, 100, 'user2', '/share2', 31], + [2, 100, 'user2', '/share2', 31, []], ], [ - [1, 100, 'nullgroup', '/share2-renamed', 31], + [1, 100, 'nullgroup', '/share2-renamed', 31, []], ], [ // use target of least recent share - ['1', 100, 'nullgroup', '/share2-renamed', 31], + ['1', 100, 'nullgroup', '/share2-renamed', 31, []], ], true ], @@ -348,10 +377,10 @@ class MountProviderTest extends \Test\TestCase { $userManager = $this->createMock(IUserManager::class); $userShares = array_map(function ($shareSpec) { - return $this->makeMockShare($shareSpec[0], $shareSpec[1], $shareSpec[2], $shareSpec[3], $shareSpec[4]); + return $this->makeMockShare($shareSpec[0], $shareSpec[1], $shareSpec[2], $shareSpec[3], $shareSpec[4], $shareSpec[5]); }, $userShares); $groupShares = array_map(function ($shareSpec) { - return $this->makeMockShare($shareSpec[0], $shareSpec[1], $shareSpec[2], $shareSpec[3], $shareSpec[4]); + return $this->makeMockShare($shareSpec[0], $shareSpec[1], $shareSpec[2], $shareSpec[3], $shareSpec[4], $shareSpec[5]); }, $groupShares); $this->user->expects($this->any()) @@ -410,6 +439,11 @@ class MountProviderTest extends \Test\TestCase { $this->assertEquals($expectedShare[2], $share->getShareOwner()); $this->assertEquals($expectedShare[3], $share->getTarget()); $this->assertEquals($expectedShare[4], $share->getPermissions()); + if ($expectedShare[5] === null) { + $this->assertNull($share->getAttributes()); + } else { + $this->assertEquals($expectedShare[5], $share->getAttributes()->toArray()); + } } } } diff --git a/apps/settings/lib/Controller/ChangePasswordController.php b/apps/settings/lib/Controller/ChangePasswordController.php index 8dd1e6ba028..f595368563f 100644 --- a/apps/settings/lib/Controller/ChangePasswordController.php +++ b/apps/settings/lib/Controller/ChangePasswordController.php @@ -107,7 +107,7 @@ class ChangePasswordController extends Controller { } try { - if ($newpassword === null || $user->setPassword($newpassword) === false) { + if ($newpassword === null || strlen($newpassword) > 469 || $user->setPassword($newpassword) === false) { return new JSONResponse([ 'status' => 'error' ]); @@ -155,6 +155,16 @@ class ChangePasswordController extends Controller { ]); } + if (strlen($password) > 469) { + return new JSONResponse([ + 'status' => 'error', + 'data' => [ + 'message' => $this->l->t('Unable to change password. Password too long.'), + ], + ]); + } + + $currentUser = $this->userSession->getUser(); $targetUser = $this->userManager->get($username); if ($currentUser === null || $targetUser === null || |