diff options
5 files changed, 128 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 b0d7781515f..d48e7671cfa 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)) { @@ -165,7 +158,7 @@ class ShareController extends Controller { $file = basename($originalSharePath); - $shareTmpl = array(); + $shareTmpl = []; $shareTmpl['displayName'] = User::getDisplayName($shareOwner); $shareTmpl['filename'] = $file; $shareTmpl['directory_path'] = $linkItem['file_target']; @@ -289,22 +282,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 189fb57653c..204422853d0 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 \Test\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 \Test\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 \Test\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 @@ -176,4 +184,54 @@ class ShareControllerTest extends \Test\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); + } + } diff --git a/lib/public/appframework/http/notfoundresponse.php b/lib/public/appframework/http/notfoundresponse.php new file mode 100644 index 00000000000..21f0461f5e6 --- /dev/null +++ b/lib/public/appframework/http/notfoundresponse.php @@ -0,0 +1,43 @@ +<?php +/** + * @author Lukas Reschke <lukas@owncloud.com> + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @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 OCP\AppFramework\Http; + +use OCP\AppFramework\Http; +use OCP\Template; + +/** + * A generic 404 response showing an 404 error page as well to the end-user + */ +class NotFoundResponse extends Response { + + public function __construct() { + $this->setStatus(404); + } + + /** + * @return string + */ + public function render() { + $template = new Template('core', '404', 'guest'); + return $template->fetchPage(); + } +} |