Skip to content

Commit

Permalink
Merge pull request #20928 from owncloud/publicdav-check-permissions
Browse files Browse the repository at this point in the history
Check that the owner of a link share still has share permissions on access
  • Loading branch information
C. Montero Luque committed Feb 9, 2016
2 parents 962d0c3 + acd8c72 commit f64dbc6
Show file tree
Hide file tree
Showing 5 changed files with 139 additions and 2 deletions.
8 changes: 7 additions & 1 deletion apps/dav/appinfo/v1/publicwebdav.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@

$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
Expand All @@ -68,9 +70,13 @@
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();
2 changes: 1 addition & 1 deletion apps/dav/lib/connector/sabre/serverfactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ public function createServer($baseUri,
$userFolder = \OC::$server->getUserFolder();

/** @var \OC\Files\View $view */
$view = $viewCallBack();
$view = $viewCallBack($server);
$rootInfo = $view->getFileInfo('');

// Create ownCloud Dir
Expand Down
63 changes: 63 additions & 0 deletions apps/dav/lib/files/sharing/publiclinkcheckplugin.php
Original file line number Diff line number Diff line change
@@ -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();
}
}
}
17 changes: 17 additions & 0 deletions apps/files_sharing/lib/controllers/sharecontroller.php
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,16 @@ protected function emitAccessShareHook($share, $errorCode = 200, $errorMessage =
}
}

/**
* 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
Expand All @@ -253,6 +263,9 @@ public function showShare($token, $path = '') {
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 !== '') {
Expand Down Expand Up @@ -371,6 +384,10 @@ public function downloadShare($token, $files = null, $path = '', $downloadStartS
$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
Expand Down
51 changes: 51 additions & 0 deletions apps/files_sharing/tests/controller/sharecontroller.php
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,8 @@ public function testShowShare() {
$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);
Expand Down Expand Up @@ -363,6 +365,55 @@ public function testShowShare() {
$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');
Expand Down

0 comments on commit f64dbc6

Please sign in to comment.