diff options
author | Vincent Petry <pvince81@owncloud.com> | 2015-03-26 17:25:07 +0100 |
---|---|---|
committer | Vincent Petry <pvince81@owncloud.com> | 2015-03-26 17:25:07 +0100 |
commit | bc7676089c7e5fe44d4676b51722f4b6c73b11a1 (patch) | |
tree | 0941c780120abd962591d4521b7742b6953c622a /apps/files_sharing | |
parent | c0bcaa49804eb82fa23f3624e68b7e62c8ac687a (diff) | |
parent | f7f14708a2b1dd9695187a80ebf04ad4a1dea9a8 (diff) | |
download | nextcloud-server-bc7676089c7e5fe44d4676b51722f4b6c73b11a1.tar.gz nextcloud-server-bc7676089c7e5fe44d4676b51722f4b6c73b11a1.zip |
Merge pull request #15193 from owncloud/stable8-15145
Stable8: 15145
Diffstat (limited to 'apps/files_sharing')
4 files changed, 87 insertions, 23 deletions
diff --git a/apps/files_sharing/application.php b/apps/files_sharing/application.php index 3302848106f..e23960cf2bb 100644 --- a/apps/files_sharing/application.php +++ b/apps/files_sharing/application.php @@ -42,7 +42,7 @@ class Application extends App { $server->getAppConfig(), $server->getConfig(), $c->query('URLGenerator'), - $server->getUserManager(), + $c->query('UserManager'), $server->getLogger(), $server->getActivityManager() ); @@ -65,6 +65,9 @@ class Application extends App { $container->registerService('URLGenerator', function(SimpleContainer $c) use ($server){ return $server->getUrlGenerator(); }); + $container->registerService('UserManager', function(SimpleContainer $c) use ($server){ + return $server->getUserManager(); + }); $container->registerService('IsIncomingShareEnabled', function(SimpleContainer $c) { return Helper::isIncomingServer2serverShareEnabled(); }); diff --git a/apps/files_sharing/lib/controllers/sharecontroller.php b/apps/files_sharing/lib/controllers/sharecontroller.php index cd013d4ca96..b224b7dc108 100644 --- a/apps/files_sharing/lib/controllers/sharecontroller.php +++ b/apps/files_sharing/lib/controllers/sharecontroller.php @@ -17,12 +17,12 @@ use OC_Files; use OC_Util; use OCP; use OCP\Template; -use OCP\JSON; use OCP\Share; use OCP\AppFramework\Controller; use OCP\IRequest; use OCP\AppFramework\Http\TemplateResponse; use OCP\AppFramework\Http\RedirectResponse; +use OCP\AppFramework\Http\NotFoundResponse; use OC\URLGenerator; use OC\AppConfig; use OCP\ILogger; @@ -60,7 +60,7 @@ class ShareController extends Controller { * @param AppConfig $appConfig * @param OCP\IConfig $config * @param URLGenerator $urlGenerator - * @param OC\User\Manager $userManager + * @param OCP\IUserManager $userManager * @param ILogger $logger * @param OCP\Activity\IManager $activityManager */ @@ -70,7 +70,7 @@ class ShareController extends Controller { AppConfig $appConfig, OCP\IConfig $config, URLGenerator $urlGenerator, - OC\User\Manager $userManager, + OCP\IUserManager $userManager, ILogger $logger, OCP\Activity\IManager $activityManager) { parent::__construct($appName, $request); @@ -113,7 +113,7 @@ class ShareController extends Controller { public function authenticate($token, $password = '') { $linkItem = Share::getShareByToken($token, false); if($linkItem === false) { - return new TemplateResponse('core', '404', array(), 'guest'); + return new NotFoundResponse(); } $authenticate = Helper::authenticate($linkItem, $password); @@ -139,18 +139,11 @@ class ShareController extends Controller { // Check whether share exists $linkItem = Share::getShareByToken($token, false); if($linkItem === false) { - return new TemplateResponse('core', '404', array(), 'guest'); + return new NotFoundResponse(); } $shareOwner = $linkItem['uid_owner']; - $originalSharePath = null; - $rootLinkItem = OCP\Share::resolveReShare($linkItem); - if (isset($rootLinkItem['uid_owner'])) { - OCP\JSON::checkUserExists($rootLinkItem['uid_owner']); - OC_Util::tearDownFS(); - OC_Util::setupFS($rootLinkItem['uid_owner']); - $originalSharePath = Filesystem::getPath($linkItem['file_source']); - } + $originalSharePath = $this->getPath($token); // Share is password protected - check whether the user is permitted to access the share if (isset($linkItem['share_with']) && !Helper::authenticate($linkItem)) { @@ -161,11 +154,13 @@ class ShareController extends Controller { if (Filesystem::isReadable($originalSharePath . $path)) { $getPath = Filesystem::normalizePath($path); $originalSharePath .= $path; + } else { + throw new OCP\Files\NotFoundException(); } $file = basename($originalSharePath); - $shareTmpl = array(); + $shareTmpl = []; $shareTmpl['displayName'] = User::getDisplayName($shareOwner); $shareTmpl['filename'] = $file; $shareTmpl['directory_path'] = $linkItem['file_target']; @@ -263,22 +258,29 @@ class ShareController extends Controller { } /** - * @param $token - * @return null|string + * @param string $token + * @return string Resolved file path of the token + * @throws \Exception In case share could not get properly resolved */ private function getPath($token) { $linkItem = Share::getShareByToken($token, false); - $path = null; if (is_array($linkItem) && isset($linkItem['uid_owner'])) { // seems to be a valid share $rootLinkItem = Share::resolveReShare($linkItem); if (isset($rootLinkItem['uid_owner'])) { - JSON::checkUserExists($rootLinkItem['uid_owner']); + if(!$this->userManager->userExists($rootLinkItem['uid_owner'])) { + throw new \Exception('Owner of the share does not exist anymore'); + } OC_Util::tearDownFS(); OC_Util::setupFS($rootLinkItem['uid_owner']); $path = Filesystem::getPath($linkItem['file_source']); + + if(!empty($path) && Filesystem::isReadable($path)) { + return $path; + } } } - return $path; + + throw new \Exception('No file found belonging to file.'); } } diff --git a/apps/files_sharing/lib/middleware/sharingcheckmiddleware.php b/apps/files_sharing/lib/middleware/sharingcheckmiddleware.php index 3508407f2a0..3e7cdf4aa34 100644 --- a/apps/files_sharing/lib/middleware/sharingcheckmiddleware.php +++ b/apps/files_sharing/lib/middleware/sharingcheckmiddleware.php @@ -11,6 +11,7 @@ namespace OCA\Files_Sharing\Middleware; use OCP\App\IAppManager; +use OCP\AppFramework\Http\NotFoundResponse; use OCP\AppFramework\Middleware; use OCP\AppFramework\Http\TemplateResponse; use OCP\IConfig; @@ -59,7 +60,7 @@ class SharingCheckMiddleware extends Middleware { * @return TemplateResponse */ public function afterException($controller, $methodName, \Exception $exception){ - return new TemplateResponse('core', '404', array(), 'guest'); + return new NotFoundResponse(); } /** diff --git a/apps/files_sharing/tests/controller/sharecontroller.php b/apps/files_sharing/tests/controller/sharecontroller.php index 931cd506d43..ed17dd8e6b3 100644 --- a/apps/files_sharing/tests/controller/sharecontroller.php +++ b/apps/files_sharing/tests/controller/sharecontroller.php @@ -12,6 +12,7 @@ namespace OCA\Files_Sharing\Controllers; use OC\Files\Filesystem; use OCA\Files_Sharing\Application; +use OCP\AppFramework\Http\NotFoundResponse; use OCP\AppFramework\IAppContainer; use OCP\Files; use OCP\AppFramework\Http\RedirectResponse; @@ -49,6 +50,8 @@ class ShareControllerTest extends \PHPUnit_Framework_TestCase { ->disableOriginalConstructor()->getMock(); $this->container['URLGenerator'] = $this->getMockBuilder('\OC\URLGenerator') ->disableOriginalConstructor()->getMock(); + $this->container['UserManager'] = $this->getMockBuilder('\OCP\IUserManager') + ->disableOriginalConstructor()->getMock(); $this->urlGenerator = $this->container['URLGenerator']; $this->shareController = $this->container['ShareController']; @@ -115,7 +118,7 @@ class ShareControllerTest extends \PHPUnit_Framework_TestCase { public function testAuthenticate() { // Test without a not existing token $response = $this->shareController->authenticate('ThisTokenShouldHopefullyNeverExistSoThatTheUnitTestWillAlwaysPass :)'); - $expectedResponse = new TemplateResponse('core', '404', array(), 'guest'); + $expectedResponse = new NotFoundResponse(); $this->assertEquals($expectedResponse, $response); // Test with a valid password @@ -130,9 +133,14 @@ class ShareControllerTest extends \PHPUnit_Framework_TestCase { } public function testShowShare() { + $this->container['UserManager']->expects($this->exactly(2)) + ->method('userExists') + ->with($this->user) + ->will($this->returnValue(true)); + // Test without a not existing token $response = $this->shareController->showShare('ThisTokenShouldHopefullyNeverExistSoThatTheUnitTestWillAlwaysPass :)'); - $expectedResponse = new TemplateResponse('core', '404', array(), 'guest'); + $expectedResponse = new NotFoundResponse(); $this->assertEquals($expectedResponse, $response); // Test with a password protected share and no authentication @@ -170,4 +178,54 @@ class ShareControllerTest extends \PHPUnit_Framework_TestCase { array('token' => $this->token))); $this->assertEquals($expectedResponse, $response); } + + /** + * @expectedException \Exception + * @expectedExceptionMessage No file found belonging to file. + */ + public function testShowShareWithDeletedFile() { + $this->container['UserManager']->expects($this->once()) + ->method('userExists') + ->with($this->user) + ->will($this->returnValue(true)); + + $view = new View('/'. $this->user . '/files'); + $view->unlink('file1.txt'); + $linkItem = Share::getShareByToken($this->token, false); + \OC::$server->getSession()->set('public_link_authenticated', $linkItem['id']); + $this->shareController->showShare($this->token); + } + + /** + * @expectedException \Exception + * @expectedExceptionMessage No file found belonging to file. + */ + public function testDownloadShareWithDeletedFile() { + $this->container['UserManager']->expects($this->once()) + ->method('userExists') + ->with($this->user) + ->will($this->returnValue(true)); + + $view = new View('/'. $this->user . '/files'); + $view->unlink('file1.txt'); + $linkItem = Share::getShareByToken($this->token, false); + \OC::$server->getSession()->set('public_link_authenticated', $linkItem['id']); + $this->shareController->downloadShare($this->token); + } + + /** + * @expectedException \Exception + * @expectedExceptionMessage Owner of the share does not exist anymore + */ + public function testShowShareWithNotExistingUser() { + $this->container['UserManager']->expects($this->once()) + ->method('userExists') + ->with($this->user) + ->will($this->returnValue(false)); + + $linkItem = Share::getShareByToken($this->token, false); + \OC::$server->getSession()->set('public_link_authenticated', $linkItem['id']); + $this->shareController->showShare($this->token); + } + } |