Despite it's PHPDoc the function might return `null` which was not properly catched and thus in some situations the share was resolved to the sharing users root directory.
To test this perform the following steps:
* Share file in owncloud 7 (7.0.4.2)
* Delete the parent folder of the shared file
* The share stays is in the DB and the share via the sharelink is inaccessible. (which is good)
* Upgrade to owncloud 8 (8.0.2) (This step is crucial. The bug is not reproduceable without upgrading from 7 to 8. It seems like the old tokens are handled different than the newer ones)
* Optional Step: Logout, Reset Browser Session, etc.
* Access the share via the old share url: almost empty page, but there is a dowload button which adds a "/download" to the URL.
* Upon clicking, a download.zip is downloaded which contains EVERYTHING from the owncloud directory (of the user who shared the file)
* No exception is thrown and no error is logged.
This will add a check whether the share is a valid one and also adds unit tests to prevent further regressions in the future. Needs to be backported to ownCloud 8.
Adding a proper clean-up of the orphaned shares is out-of-scope and would probably require some kind of FK or so.
Fixes https://github.com/owncloud/core/issues/15097
$server->getAppConfig(),
$server->getConfig(),
$c->query('URLGenerator'),
- $server->getUserManager(),
+ $c->query('UserManager'),
$server->getLogger(),
$server->getActivityManager()
);
$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();
});
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;
* @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
*/
AppConfig $appConfig,
OCP\IConfig $config,
URLGenerator $urlGenerator,
- OC\User\Manager $userManager,
+ OCP\IUserManager $userManager,
ILogger $logger,
OCP\Activity\IManager $activityManager) {
parent::__construct($appName, $request);
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);
// 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)) {
$file = basename($originalSharePath);
- $shareTmpl = array();
+ $shareTmpl = [];
$shareTmpl['displayName'] = User::getDisplayName($shareOwner);
$shareTmpl['filename'] = $file;
$shareTmpl['directory_path'] = $linkItem['file_target'];
}
/**
- * @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.');
}
}
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;
* @return TemplateResponse
*/
public function afterException($controller, $methodName, \Exception $exception){
- return new TemplateResponse('core', '404', array(), 'guest');
+ return new NotFoundResponse();
}
/**
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;
->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'];
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
}
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
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);
+ }
+
}
--- /dev/null
+<?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();
+ }
+}
projects / nextcloud-server.git / commitdiff