Skip to content

Commit

Permalink
Adding new Symfony events for zipfile download and public link share
Browse files Browse the repository at this point in the history
Adding new Symfony events, to check, if creating
shares or downloading zips are blocked by any
apps or not.

Signed-off-by: Sujith H <[email protected]>
  • Loading branch information
sharidas committed Dec 22, 2017
1 parent 22f5f1e commit 89c1dc3
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 6 deletions.
26 changes: 24 additions & 2 deletions apps/files_sharing/lib/API/Share20OCS.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -69,6 +71,9 @@ class Share20OCS {
*/
private $additionalInfoField;

/** @var \Symfony\Component\EventDispatcher\EventDispatcherInterface */
private $eventDispatcher;

/**
* Share20OCS constructor.
*
Expand All @@ -81,6 +86,7 @@ class Share20OCS {
* @param IUser $currentUser
* @param IL10N $l10n
* @param IConfig $config
* @param EventDispatcher $eventDispatcher
*/
public function __construct(
IManager $shareManager,
Expand All @@ -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;
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -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()) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
74 changes: 71 additions & 3 deletions apps/files_sharing/tests/API/Share20OCSTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -36,6 +37,7 @@
use OCP\IUserManager;
use OCP\Lock\LockedException;
use OCP\Share;
use Symfony\Component\EventDispatcher\GenericEvent;
use Test\TestCase;

/**
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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,
Expand All @@ -112,7 +119,8 @@ protected function setUp() {
$this->urlGenerator,
$this->currentUser,
$this->l,
$this->config
$this->config,
$this->eventDispatcher
);
}

Expand All @@ -128,6 +136,7 @@ private function mockFormatShare() {
$this->currentUser,
$this->l,
$this->config,
$this->eventDispatcher,
])->setMethods(['formatShare'])
->getMock();
}
Expand Down Expand Up @@ -436,6 +445,7 @@ public function testGetShare(\OCP\Share\IShare $share, array $result) {
$this->currentUser,
$this->l,
$this->config,
$this->eventDispatcher,
])->setMethods(['canAccessShare'])
->getMock();

Expand Down Expand Up @@ -763,6 +773,7 @@ public function testCreateShareUser() {
$this->currentUser,
$this->l,
$this->config,
$this->eventDispatcher,
])->setMethods(['formatShare'])
->getMock();

Expand Down Expand Up @@ -815,11 +826,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() {
Expand Down Expand Up @@ -880,6 +900,7 @@ public function testCreateShareGroup() {
$this->currentUser,
$this->l,
$this->config,
$this->eventDispatcher,
])->setMethods(['formatShare'])
->getMock();

Expand Down Expand Up @@ -1417,6 +1438,7 @@ public function testCreateReshareOfFederatedMountNoDeletePermissions() {
$this->currentUser,
$this->l,
$this->config,
$this->eventDispatcher,
])->setMethods(['formatShare'])
->getMock();

Expand Down Expand Up @@ -2699,7 +2721,8 @@ public function getOcsDisabledAPI() {
$this->urlGenerator,
$this->currentUser,
$this->l,
$this->config
$this->config,
$this->eventDispatcher
);
}

Expand Down Expand Up @@ -2790,7 +2813,8 @@ public function testGetShareAdditionalInfo($configValue, $expectedInfo) {
$this->urlGenerator,
$this->currentUser,
$this->l,
$config
$config,
$this->eventDispatcher
);

list($file,) = $this->getMockFileFolder();
Expand Down Expand Up @@ -2822,4 +2846,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]);
}
}
16 changes: 15 additions & 1 deletion apps/files_sharing/tests/ApiTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
use OCP\IL10N;
use OCP\IRequest;
use OCP\Share;
use Symfony\Component\EventDispatcher\GenericEvent;

/**
* Class ApiTest
Expand Down Expand Up @@ -115,6 +116,10 @@ private function createOCS($request, $userId) {
return vsprintf($text, $parameters);
}));

\OC::$server->getEventDispatcher()->addListener('share.beforeCreate', function (GenericEvent $event) {
$event->setArgument('run', true);
});

return new \OCA\Files_Sharing\API\Share20OCS(
$this->shareManager,
\OC::$server->getGroupManager(),
Expand All @@ -124,7 +129,8 @@ private function createOCS($request, $userId) {
\OC::$server->getURLGenerator(),
$currentUser,
$l,
\OC::$server->getConfig()
\OC::$server->getConfig(),
\OC::$server->getEventDispatcher()
);
}

Expand All @@ -137,10 +143,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']);
Expand Down
13 changes: 13 additions & 0 deletions lib/private/legacy/files.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) and ($event->hasArgument('errorMessage'))) {
throw new \OC\ForbiddenException("Access denied: " . $event->getArgument('errorMessage'));
}

$streamer = new Streamer();
OC_Util::obEnd();

Expand Down Expand Up @@ -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']);
OC::$server->getEventDispatcher()->dispatch('file.afterCreateZip', $event);

} catch (\OCP\Lock\LockedException $ex) {
self::unlockAllTheFiles($dir, $files, $getType, $view, $filename);
OC::$server->getLogger()->logException($ex);
Expand All @@ -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);
}
}
Expand Down

0 comments on commit 89c1dc3

Please sign in to comment.