Skip to content

Commit

Permalink
Merge pull request #29885 from owncloud/add-symfony-events-part2
Browse files Browse the repository at this point in the history
Adding new Symfony events for zipfile download and public link share
  • Loading branch information
Vincent Petry authored Jan 10, 2018
2 parents 412f756 + 7e977b4 commit f1a45d1
Show file tree
Hide file tree
Showing 5 changed files with 128 additions and 7 deletions.
3 changes: 2 additions & 1 deletion apps/files_sharing/lib/API/OCSShareWrapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()
);
}

Expand Down
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
81 changes: 78 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,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([
Expand All @@ -128,6 +143,7 @@ private function mockFormatShare() {
$this->currentUser,
$this->l,
$this->config,
$this->eventDispatcher,
])->setMethods(['formatShare'])
->getMock();
}
Expand Down Expand Up @@ -436,6 +452,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 +780,7 @@ public function testCreateShareUser() {
$this->currentUser,
$this->l,
$this->config,
$this->eventDispatcher,
])->setMethods(['formatShare'])
->getMock();

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

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

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

Expand Down Expand Up @@ -2790,7 +2820,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 +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]);
}
}
12 changes: 11 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 @@ -124,7 +125,8 @@ private function createOCS($request, $userId) {
\OC::$server->getURLGenerator(),
$currentUser,
$l,
\OC::$server->getConfig()
\OC::$server->getConfig(),
\OC::$server->getEventDispatcher()
);
}

Expand All @@ -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']);
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) or ($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', '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);
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 f1a45d1

Please sign in to comment.