From 7e977b4337fea8cdc34542f5a3e84a6ef43208f7 Mon Sep 17 00:00:00 2001 From: Sujith H Date: Mon, 18 Dec 2017 20:12:37 +0530 Subject: [PATCH] Adding new Symfony events for zipfile download and public link share Adding new Symfony events, to check, if creating shares or downloading zips are blocked by any apps or not. Signed-off-by: Sujith H --- .../files_sharing/lib/API/OCSShareWrapper.php | 3 +- apps/files_sharing/lib/API/Share20OCS.php | 26 +++++- .../tests/API/Share20OCSTest.php | 81 ++++++++++++++++++- apps/files_sharing/tests/ApiTest.php | 12 ++- lib/private/legacy/files.php | 13 +++ 5 files changed, 128 insertions(+), 7 deletions(-) diff --git a/apps/files_sharing/lib/API/OCSShareWrapper.php b/apps/files_sharing/lib/API/OCSShareWrapper.php index 14c9cd400124..77b0b45a8f2c 100644 --- a/apps/files_sharing/lib/API/OCSShareWrapper.php +++ b/apps/files_sharing/lib/API/OCSShareWrapper.php @@ -35,7 +35,8 @@ private function getShare20OCS() { \OC::$server->getURLGenerator(), \OC::$server->getUserSession()->getUser(), \OC::$server->getL10N('files_sharing'), - \OC::$server->getConfig() + \OC::$server->getConfig(), + \OC::$server->getEventDispatcher() ); } diff --git a/apps/files_sharing/lib/API/Share20OCS.php b/apps/files_sharing/lib/API/Share20OCS.php index f23d5180b783..dab285978e16 100644 --- a/apps/files_sharing/lib/API/Share20OCS.php +++ b/apps/files_sharing/lib/API/Share20OCS.php @@ -37,6 +37,8 @@ use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IManager; use OCP\Share\IShare; +use Symfony\Component\EventDispatcher\EventDispatcher; +use Symfony\Component\EventDispatcher\GenericEvent; /** * Class Share20OCS @@ -69,6 +71,9 @@ class Share20OCS { */ private $additionalInfoField; + /** @var \Symfony\Component\EventDispatcher\EventDispatcherInterface */ + private $eventDispatcher; + /** * Share20OCS constructor. * @@ -81,6 +86,7 @@ class Share20OCS { * @param IUser $currentUser * @param IL10N $l10n * @param IConfig $config + * @param EventDispatcher $eventDispatcher */ public function __construct( IManager $shareManager, @@ -91,7 +97,8 @@ public function __construct( IURLGenerator $urlGenerator, IUser $currentUser, IL10N $l10n, - IConfig $config + IConfig $config, + EventDispatcher $eventDispatcher ) { $this->shareManager = $shareManager; $this->userManager = $userManager; @@ -103,6 +110,7 @@ public function __construct( $this->l = $l10n; $this->config = $config; $this->additionalInfoField = $this->config->getAppValue('core', 'user_additional_info_field', ''); + $this->eventDispatcher = $eventDispatcher; } /** @@ -272,6 +280,7 @@ public function deleteShare($id) { * @return \OC\OCS\Result */ public function createShare() { + $event = new GenericEvent(null); $share = $this->shareManager->newShare(); if (!$this->shareManager->shareApiEnabled()) { @@ -287,6 +296,15 @@ public function createShare() { } $userFolder = $this->rootFolder->getUserFolder($this->currentUser->getUID()); + $event->setArgument('userFolder', $userFolder); + $event->setArgument('path', $path); + $event->setArgument('name', $name); + $event->setArgument('run', true); + //Dispatch an event to see if creating shares is blocked by any app + $this->eventDispatcher->dispatch('share.beforeCreate', $event); + if ($event->getArgument('run') === false) { + return new \OC\OCS\Result(null, 403, $this->l->t('No permission to create share')); + } try { $path = $userFolder->get($path); } catch (NotFoundException $e) { @@ -466,12 +484,16 @@ public function createShare() { $share->getNode()->unlock(\OCP\Lock\ILockingProvider::LOCK_SHARED); + $event = new GenericEvent(null, []); + $event->setArgument('output', $output); + $event->setArgument('result', 'success'); + $this->eventDispatcher->dispatch('share.afterCreate', $event); return new \OC\OCS\Result($output); } /** * @param \OCP\Files\File|\OCP\Files\Folder $node - * @param boolean $includeTags + * @param boolean $includeTags * @return \OC\OCS\Result */ private function getSharedWithMe($node = null, $includeTags) { diff --git a/apps/files_sharing/tests/API/Share20OCSTest.php b/apps/files_sharing/tests/API/Share20OCSTest.php index 0fbce3247bec..bca6af9873c9 100644 --- a/apps/files_sharing/tests/API/Share20OCSTest.php +++ b/apps/files_sharing/tests/API/Share20OCSTest.php @@ -24,6 +24,7 @@ */ namespace OCA\Files_Sharing\Tests\API; +use JMS\Serializer\EventDispatcher\EventDispatcher; use OCA\Files_Sharing\API\Share20OCS; use OCP\Files\IRootFolder; use OCP\Files\NotFoundException; @@ -36,6 +37,7 @@ use OCP\IUserManager; use OCP\Lock\LockedException; use OCP\Share; +use Symfony\Component\EventDispatcher\GenericEvent; use Test\TestCase; /** @@ -73,6 +75,9 @@ class Share20OCSTest extends TestCase { /** @var IL10N */ private $l; + /** @var EventDispatcher */ + private $eventDispatcher; + protected function setUp() { $this->shareManager = $this->getMockBuilder('OCP\Share\IManager') ->disableOriginalConstructor() @@ -103,6 +108,8 @@ protected function setUp() { ['core', 'shareapi_default_permissions', \OCP\Constants::PERMISSION_ALL, \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE] ])); + $this->eventDispatcher = \OC::$server->getEventDispatcher(); + $this->ocs = new Share20OCS( $this->shareManager, $this->groupManager, @@ -112,10 +119,18 @@ protected function setUp() { $this->urlGenerator, $this->currentUser, $this->l, - $this->config + $this->config, + $this->eventDispatcher ); } + protected function tearDown() { + \OC::$server->getEventDispatcher()->addListener('share.beforeCreate', function (GenericEvent $event) { + $event->setArgument('run', true); + }); + return parent::tearDown(); + } + private function mockFormatShare() { return $this->getMockBuilder('OCA\Files_Sharing\API\Share20OCS') ->setConstructorArgs([ @@ -128,6 +143,7 @@ private function mockFormatShare() { $this->currentUser, $this->l, $this->config, + $this->eventDispatcher, ])->setMethods(['formatShare']) ->getMock(); } @@ -436,6 +452,7 @@ public function testGetShare(\OCP\Share\IShare $share, array $result) { $this->currentUser, $this->l, $this->config, + $this->eventDispatcher, ])->setMethods(['canAccessShare']) ->getMock(); @@ -763,6 +780,7 @@ public function testCreateShareUser() { $this->currentUser, $this->l, $this->config, + $this->eventDispatcher, ])->setMethods(['formatShare']) ->getMock(); @@ -815,11 +833,20 @@ public function testCreateShareUser() { })) ->will($this->returnArgument(0)); + $calledAfterCreate = []; + \OC::$server->getEventDispatcher()->addListener('share.afterCreate', function (GenericEvent $event) use (&$calledAfterCreate) { + $calledAfterCreate[] = 'share.afterCreate'; + $calledAfterCreate[] = $event; + }); $expected = new \OC\OCS\Result(); $result = $ocs->createShare(); $this->assertEquals($expected->getMeta(), $result->getMeta()); $this->assertEquals($expected->getData(), $result->getData()); + $this->assertEquals('share.afterCreate', $calledAfterCreate[0]); + $this->assertInstanceOf(GenericEvent::class, $calledAfterCreate[1]); + $this->assertEquals('success', $calledAfterCreate[1]->getArgument('result')); + $this->assertArrayHasKey('output', $calledAfterCreate[1]); } public function testCreateShareGroupNoValidShareWith() { @@ -880,6 +907,7 @@ public function testCreateShareGroup() { $this->currentUser, $this->l, $this->config, + $this->eventDispatcher, ])->setMethods(['formatShare']) ->getMock(); @@ -1417,6 +1445,7 @@ public function testCreateReshareOfFederatedMountNoDeletePermissions() { $this->currentUser, $this->l, $this->config, + $this->eventDispatcher, ])->setMethods(['formatShare']) ->getMock(); @@ -2699,7 +2728,8 @@ public function getOcsDisabledAPI() { $this->urlGenerator, $this->currentUser, $this->l, - $this->config + $this->config, + $this->eventDispatcher ); } @@ -2790,7 +2820,8 @@ public function testGetShareAdditionalInfo($configValue, $expectedInfo) { $this->urlGenerator, $this->currentUser, $this->l, - $config + $config, + $this->eventDispatcher ); list($file,) = $this->getMockFileFolder(); @@ -2822,4 +2853,48 @@ public function testGetShareAdditionalInfo($configValue, $expectedInfo) { $this->assertEquals($expectedInfo, $result['share_with_additional_info']); } + + public function testCreateShareNotAllowed() { + + $share = $this->newShare(); + $this->shareManager->method('newShare')->willReturn($share); + + $this->request + ->method('getParam') + ->will($this->returnValueMap([ + ['path', null, 'valid-path'], + ['permissions', null, \OCP\Constants::PERMISSION_ALL], + ['shareType', $this->any(), Share::SHARE_TYPE_USER], + ['shareWith', $this->any(), 'invalidUser'], + ])); + + $userFolder = $this->createMock('\OCP\Files\Folder'); + $this->rootFolder->expects($this->once()) + ->method('getUserFolder') + ->with('currentUser') + ->willReturn($userFolder); + + $path = $this->createMock('\OCP\Files\File'); + $storage = $this->createMock('OCP\Files\Storage'); + $storage->method('instanceOfStorage') + ->with('OCA\Files_Sharing\External\Storage') + ->willReturn(false); + $path->method('getStorage')->willReturn($storage); + + $expected = new \OC\OCS\Result(null, 403, 'No permission to create share'); + + $calledCreateEvent = []; + \OC::$server->getEventDispatcher()->addListener('share.beforeCreate', function (GenericEvent $event) use (&$calledCreateEvent) { + $calledCreateEvent[] = "share.beforeCreate"; + $event->setArgument('run', false); + $calledCreateEvent[] = $event; + }); + $result = $this->ocs->createShare(); + + $this->assertEquals($expected->getMeta(), $result->getMeta()); + $this->assertEquals($expected->getData(), $result->getData()); + $this->assertEquals('share.beforeCreate', $calledCreateEvent[0]); + $this->assertInstanceOf(GenericEvent::class, $calledCreateEvent[1]); + $this->assertArrayHasKey('run', $calledCreateEvent[1]); + } } diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php index 20c0f2f583b1..9de5206cf02f 100644 --- a/apps/files_sharing/tests/ApiTest.php +++ b/apps/files_sharing/tests/ApiTest.php @@ -32,6 +32,7 @@ use OCP\IL10N; use OCP\IRequest; use OCP\Share; +use Symfony\Component\EventDispatcher\GenericEvent; /** * Class ApiTest @@ -124,7 +125,8 @@ private function createOCS($request, $userId) { \OC::$server->getURLGenerator(), $currentUser, $l, - \OC::$server->getConfig() + \OC::$server->getConfig(), + \OC::$server->getEventDispatcher() ); } @@ -137,10 +139,18 @@ function testCreateShareUserFile() { $data['shareWith'] = self::TEST_FILES_SHARING_API_USER2; $data['shareType'] = Share::SHARE_TYPE_USER; + $calledBeforeCreate = []; + \OC::$server->getEventDispatcher()->addListener('share.beforeCreate', function (GenericEvent $event) use (&$calledBeforeCreate) { + $calledBeforeCreate[] = 'share.beforeCreate'; + $calledBeforeCreate[] = $event; + }); $request = $this->createRequest($data); $ocs = $this->createOCS($request, self::TEST_FILES_SHARING_API_USER1); $result = $ocs->createShare(); + $this->assertEquals('share.beforeCreate', $calledBeforeCreate[0]); + $this->assertInstanceOf(GenericEvent::class, $calledBeforeCreate[1]); + $this->assertTrue($calledBeforeCreate[1]->getArgument('run')); $this->assertTrue($result->succeeded()); $data = $result->getData(); $this->assertEquals(19, $data['permissions']); diff --git a/lib/private/legacy/files.php b/lib/private/legacy/files.php index 130951363a47..e8aa086ac98d 100644 --- a/lib/private/legacy/files.php +++ b/lib/private/legacy/files.php @@ -139,6 +139,13 @@ public static function get($dir, $files, $params = null) { } } + //Dispatch an event to see if any apps have problem with download + $event = new \Symfony\Component\EventDispatcher\GenericEvent(null, ['dir' => $dir, 'files' => $files, 'run' => true]); + OC::$server->getEventDispatcher()->dispatch('file.beforeCreateZip', $event); + if (($event->getArgument('run') === false) or ($event->hasArgument('errorMessage'))) { + throw new \OC\ForbiddenException("Access denied: " . $event->getArgument('errorMessage')); + } + $streamer = new Streamer(); OC_Util::obEnd(); @@ -167,6 +174,9 @@ public static function get($dir, $files, $params = null) { $streamer->finalize(); set_time_limit($executionTime); self::unlockAllTheFiles($dir, $files, $getType, $view, $filename); + $event = new \Symfony\Component\EventDispatcher\GenericEvent(null, ['result' => 'success', 'dir' => $dir, 'files' => $files]); + OC::$server->getEventDispatcher()->dispatch('file.afterCreateZip', $event); + } catch (\OCP\Lock\LockedException $ex) { self::unlockAllTheFiles($dir, $files, $getType, $view, $filename); OC::$server->getLogger()->logException($ex); @@ -183,6 +193,9 @@ public static function get($dir, $files, $params = null) { OC::$server->getLogger()->logException($ex); $l = \OC::$server->getL10N('core'); $hint = method_exists($ex, 'getHint') ? $ex->getHint() : ''; + if ($event->hasArgument('message')) { + $hint .= ' ' . $event->getArgument('message'); + } \OC_Template::printErrorPage($l->t('File cannot be read'), $hint); } }