Merge pull request #20928 from owncloud/publicdav-check-permissions

Check that the owner of a link share still has share permissions on access

5 files changed, 139 insertions, 2 deletions

diff --git a/apps/dav/appinfo/v1/publicwebdav.php b/apps/dav/appinfo/v1/publicwebdav.php
index 6ddb570aca8..b0ee264aac3 100644
--- a/apps/dav/appinfo/v1/publicwebdav.php
+++ b/apps/dav/appinfo/v1/publicwebdav.php
@@ -46,7 +46,9 @@ $serverFactory = new OCA\DAV\Connector\Sabre\ServerFactory(
 
 $requestUri = \OC::$server->getRequest()->getRequestUri();
 
-$server = $serverFactory->createServer($baseuri, $requestUri, $authBackend, function () use ($authBackend) {
+$linkCheckPlugin = new \OCA\DAV\Files\Sharing\PublicLinkCheckPlugin();
+
+$server = $serverFactory->createServer($baseuri, $requestUri, $authBackend, function (\Sabre\DAV\Server $server) use ($authBackend, $linkCheckPlugin) {
 	$isAjax = (isset($_SERVER['HTTP_X_REQUESTED_WITH']) && $_SERVER['HTTP_X_REQUESTED_WITH'] === 'XMLHttpRequest');
 	if (OCA\Files_Sharing\Helper::isOutgoingServer2serverShareEnabled() === false && !$isAjax) {
 		// this is what is thrown when trying to access a non-existing share
@@ -68,9 +70,13 @@ $server = $serverFactory->createServer($baseuri, $requestUri, $authBackend, func
 	OC_Util::setupFS($owner);
 	$ownerView = \OC\Files\Filesystem::getView();
 	$path = $ownerView->getPath($fileId);
+	$fileInfo = $ownerView->getFileInfo($path);
+	$linkCheckPlugin->setFileInfo($fileInfo);
 
 	return new \OC\Files\View($ownerView->getAbsolutePath($path));
 });
 
+$server->addPlugin($linkCheckPlugin);
+
 // And off we go!
 $server->exec();
diff --git a/apps/dav/lib/connector/sabre/serverfactory.php b/apps/dav/lib/connector/sabre/serverfactory.php
index 9a828787a0d..8253948d96f 100644
--- a/apps/dav/lib/connector/sabre/serverfactory.php
+++ b/apps/dav/lib/connector/sabre/serverfactory.php
@@ -118,7 +118,7 @@ class ServerFactory {
 			$userFolder = \OC::$server->getUserFolder();
 			
 			/** @var \OC\Files\View $view */
-			$view = $viewCallBack();
+			$view = $viewCallBack($server);
 			$rootInfo = $view->getFileInfo('');
 
 			// Create ownCloud Dir
diff --git a/apps/dav/lib/files/sharing/publiclinkcheckplugin.php b/apps/dav/lib/files/sharing/publiclinkcheckplugin.php
new file mode 100644
index 00000000000..bbb5c611204
--- /dev/null
+++ b/apps/dav/lib/files/sharing/publiclinkcheckplugin.php
@@ -0,0 +1,63 @@
+<?php
+/**
+ * @author Robin Appelman <icewind@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 OCA\DAV\Files\Sharing;
+
+use OCP\Files\FileInfo;
+use Sabre\DAV\Exception\NotFound;
+use Sabre\DAV\ServerPlugin;
+use Sabre\HTTP\RequestInterface;
+use Sabre\HTTP\ResponseInterface;
+
+/**
+ * Verify that the public link share is valid
+ */
+class PublicLinkCheckPlugin extends ServerPlugin {
+	/**
+	 * @var FileInfo
+	 */
+	private $fileInfo;
+
+	/**
+	 * @param FileInfo $fileInfo
+	 */
+	public function setFileInfo($fileInfo) {
+		$this->fileInfo = $fileInfo;
+	}
+
+	/**
+	 * This initializes the plugin.
+	 *
+	 * @param \Sabre\DAV\Server $server Sabre server
+	 *
+	 * @return void
+	 */
+	public function initialize(\Sabre\DAV\Server $server) {
+		$server->on('beforeMethod', [$this, 'beforeMethod']);
+	}
+
+	public function beforeMethod(RequestInterface $request, ResponseInterface $response){
+		// verify that the owner didn't have his share permissions revoked
+		if ($this->fileInfo && !$this->fileInfo->isShareable()) {
+			throw new NotFound();
+		}
+	}
+}
diff --git a/apps/files_sharing/lib/controllers/sharecontroller.php b/apps/files_sharing/lib/controllers/sharecontroller.php
index dae61a3537b..08679c88bb1 100644
--- a/apps/files_sharing/lib/controllers/sharecontroller.php
+++ b/apps/files_sharing/lib/controllers/sharecontroller.php
@@ -228,6 +228,16 @@ class ShareController extends Controller {
 	}
 
 	/**
+	 * Validate the permissions of the share
+	 *
+	 * @param Share\IShare $share
+	 * @return bool
+	 */
+	private function validateShare(\OCP\Share\IShare $share) {
+		return $share->getNode()->isReadable() && $share->getNode()->isShareable();
+	}
+
+	/**
 	 * @PublicPage
 	 * @NoCSRFRequired
 	 *
@@ -253,6 +263,9 @@ class ShareController extends Controller {
 				array('token' => $token)));
 		}
 
+		if (!$this->validateShare($share)) {
+			throw new NotFoundException();
+		}
 		// We can't get the path of a file share
 		try {
 			if ($share->getNode() instanceof \OCP\Files\File && $path !== '') {
@@ -371,6 +384,10 @@ class ShareController extends Controller {
 		$userFolder = $this->rootFolder->getUserFolder($share->getShareOwner());
 		$originalSharePath = $userFolder->getRelativePath($share->getNode()->getPath());
 
+		if (!$this->validateShare($share)) {
+			throw new NotFoundException();
+		}
+
 		// Single file share
 		if ($share->getNode() instanceof \OCP\Files\File) {
 			// Single file download
diff --git a/apps/files_sharing/tests/controller/sharecontroller.php b/apps/files_sharing/tests/controller/sharecontroller.php
index 11dc082390c..6167b281fce 100644
--- a/apps/files_sharing/tests/controller/sharecontroller.php
+++ b/apps/files_sharing/tests/controller/sharecontroller.php
@@ -304,6 +304,8 @@ class ShareControllerTest extends \Test\TestCase {
 		$file->method('getName')->willReturn('file1.txt');
 		$file->method('getMimetype')->willReturn('text/plain');
 		$file->method('getSize')->willReturn(33);
+		$file->method('isReadable')->willReturn(true);
+		$file->method('isShareable')->willReturn(true);
 
 		$share = \OC::$server->getShareManager()->newShare();
 		$share->setId(42);
@@ -363,6 +365,55 @@ class ShareControllerTest extends \Test\TestCase {
 		$this->assertEquals($expectedResponse, $response);
 	}
 
+	/**
+	 * @expectedException \OCP\Files\NotFoundException
+	 */
+	public function testShowShareInvalid() {
+		$owner = $this->getMock('OCP\IUser');
+		$owner->method('getDisplayName')->willReturn('ownerDisplay');
+		$owner->method('getUID')->willReturn('ownerUID');
+
+		$file = $this->getMock('OCP\Files\File');
+		$file->method('getName')->willReturn('file1.txt');
+		$file->method('getMimetype')->willReturn('text/plain');
+		$file->method('getSize')->willReturn(33);
+		$file->method('isShareable')->willReturn(false);
+		$file->method('isReadable')->willReturn(true);
+
+		$share = \OC::$server->getShareManager()->newShare();
+		$share->setId(42);
+		$share->setPassword('password')
+			->setShareOwner('ownerUID')
+			->setNode($file)
+			->setTarget('/file1.txt');
+
+		$this->session->method('exists')->with('public_link_authenticated')->willReturn(true);
+		$this->session->method('get')->with('public_link_authenticated')->willReturn('42');
+
+		$this->previewManager->method('isMimeSupported')->with('text/plain')->willReturn(true);
+
+		$this->config->method('getSystemValue')
+			->willReturnMap(
+				[
+					['max_filesize_animated_gifs_public_sharing', 10, 10],
+					['enable_previews', true, true],
+				]
+			);
+		$shareTmpl['maxSizeAnimateGif'] = $this->config->getSystemValue('max_filesize_animated_gifs_public_sharing', 10);
+		$shareTmpl['previewEnabled'] = $this->config->getSystemValue('enable_previews', true);
+
+		$this->shareManager
+			->expects($this->once())
+			->method('getShareByToken')
+			->with('token')
+			->willReturn($share);
+
+		$this->userManager->method('get')->with('ownerUID')->willReturn($owner);
+
+		$this->shareController->showShare('token');
+	}
+
+
 	public function testDownloadShare() {
 		$share = $this->getMock('\OCP\Share\IShare');
 		$share->method('getPassword')->willReturn('password');