Skip to content

Commit

Permalink
Merge pull request #43727 from nextcloud/artonge/fix/prevent_download…
Browse files Browse the repository at this point in the history
…_of_shares

Check share status when touching versions
  • Loading branch information
artonge authored Feb 21, 2024
2 parents c70a634 + 18b32e8 commit 7f33466
Show file tree
Hide file tree
Showing 10 changed files with 142 additions and 16 deletions.
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 @@ -161,7 +161,7 @@ public function createServer(string $baseUri,

// Allow view-only plugin for webdav requests
$server->addPlugin(new ViewOnlyPlugin(
$this->logger
$userFolder,
));

if ($this->userSession->isLoggedIn()) {
Expand Down
20 changes: 16 additions & 4 deletions apps/dav/lib/DAV/ViewOnlyPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
use OCA\DAV\Connector\Sabre\Exception\Forbidden;
use OCA\DAV\Connector\Sabre\File as DavFile;
use OCA\Files_Versions\Sabre\VersionFile;
use OCP\Files\Folder;
use OCP\Files\NotFoundException;
use Psr\Log\LoggerInterface;
use Sabre\DAV\Exception\NotFound;
use Sabre\DAV\Server;
use Sabre\DAV\ServerPlugin;
Expand All @@ -36,10 +36,12 @@
*/
class ViewOnlyPlugin extends ServerPlugin {
private ?Server $server = null;
private LoggerInterface $logger;
private ?Folder $userFolder;

public function __construct(LoggerInterface $logger) {
$this->logger = $logger;
public function __construct(
?Folder $userFolder,
) {
$this->userFolder = $userFolder;
}

/**
Expand Down Expand Up @@ -76,6 +78,16 @@ public function checkViewOnly(RequestInterface $request): bool {
$node = $davNode->getNode();
} elseif ($davNode instanceof VersionFile) {
$node = $davNode->getVersion()->getSourceFile();
$currentUserId = $this->userFolder?->getOwner()?->getUID();
// The version source file is relative to the owner storage.
// But we need the node from the current user perspective.
if ($node->getOwner()->getUID() !== $currentUserId) {
$nodes = $this->userFolder->getById($node->getId());
$node = array_pop($nodes);
if (!$node) {
throw new NotFoundException("Version file not accessible by current user");
}
}
} else {
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion apps/dav/lib/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ public function __construct(IRequest $request, string $baseUri) {

// Allow view-only plugin for webdav requests
$this->server->addPlugin(new ViewOnlyPlugin(
$logger
\OC::$server->getUserFolder(),
));

if (BrowserErrorPagePlugin::isBrowserRequest($request)) {
Expand Down
28 changes: 26 additions & 2 deletions apps/dav/tests/unit/DAV/ViewOnlyPluginTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,11 @@
use OCA\Files_Versions\Sabre\VersionFile;
use OCA\Files_Versions\Versions\IVersion;
use OCP\Files\File;
use OCP\Files\Folder;
use OCP\Files\Storage\IStorage;
use OCP\IUser;
use OCP\Share\IAttributes;
use OCP\Share\IShare;
use Psr\Log\LoggerInterface;
use Sabre\DAV\Server;
use Sabre\DAV\Tree;
use Sabre\HTTP\RequestInterface;
Expand All @@ -43,10 +44,13 @@ class ViewOnlyPluginTest extends TestCase {
private $tree;
/** @var RequestInterface | \PHPUnit\Framework\MockObject\MockObject */
private $request;
/** @var Folder | \PHPUnit\Framework\MockObject\MockObject */
private $userFolder;

public function setUp(): void {
$this->userFolder = $this->createMock(Folder::class);
$this->plugin = new ViewOnlyPlugin(
$this->createMock(LoggerInterface::class)
$this->userFolder,
);
$this->request = $this->createMock(RequestInterface::class);
$this->tree = $this->createMock(Tree::class);
Expand Down Expand Up @@ -111,6 +115,26 @@ public function testCanGet(bool $isVersion, ?bool $attrEnabled, bool $expectCanD
$davNode->expects($this->once())
->method('getVersion')
->willReturn($version);

$currentUser = $this->createMock(IUser::class);
$currentUser->expects($this->once())
->method('getUID')
->willReturn('alice');
$nodeInfo->expects($this->once())
->method('getOwner')
->willReturn($currentUser);

$nodeInfo = $this->createMock(File::class);
$owner = $this->createMock(IUser::class);
$owner->expects($this->once())
->method('getUID')
->willReturn('bob');
$this->userFolder->expects($this->once())
->method('getById')
->willReturn([$nodeInfo]);
$this->userFolder->expects($this->once())
->method('getOwner')
->willReturn($owner);
} else {
$davPath = 'files/path/to/file.odt';
$davNode = $this->createMock(DavFile::class);
Expand Down
1 change: 1 addition & 0 deletions apps/dav/tests/unit/ServerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ public function test($uri, array $plugins): void {
/** @var IRequest | \PHPUnit\Framework\MockObject\MockObject $r */
$r = $this->createMock(IRequest::class);
$r->expects($this->any())->method('getRequestUri')->willReturn($uri);
$this->loginAsUser('admin');
$s = new Server($r, '/');
$this->assertNotNull($s->server);
foreach ($plugins as $plugin) {
Expand Down
38 changes: 37 additions & 1 deletion apps/files_versions/lib/Versions/LegacyVersionsBackend.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
namespace OCA\Files_Versions\Versions;

use OC\Files\View;
use OCA\DAV\Connector\Sabre\Exception\Forbidden;
use OCA\Files_Sharing\ISharedStorage;
use OCA\Files_Sharing\SharedStorage;
use OCA\Files_Versions\Db\VersionEntity;
Expand All @@ -41,23 +42,27 @@
use OCP\Files\Storage\IStorage;
use OCP\IUser;
use OCP\IUserManager;
use OCP\IUserSession;

class LegacyVersionsBackend implements IVersionBackend, INameableVersionBackend, IDeletableVersionBackend, INeedSyncVersionBackend {
private IRootFolder $rootFolder;
private IUserManager $userManager;
private VersionsMapper $versionsMapper;
private IMimeTypeLoader $mimeTypeLoader;
private IUserSession $userSession;

public function __construct(
IRootFolder $rootFolder,
IUserManager $userManager,
VersionsMapper $versionsMapper,
IMimeTypeLoader $mimeTypeLoader
IMimeTypeLoader $mimeTypeLoader,
IUserSession $userSession,
) {
$this->rootFolder = $rootFolder;
$this->userManager = $userManager;
$this->versionsMapper = $versionsMapper;
$this->mimeTypeLoader = $mimeTypeLoader;
$this->userSession = $userSession;
}

public function useBackendForStorage(IStorage $storage): bool {
Expand Down Expand Up @@ -173,6 +178,10 @@ public function createVersion(IUser $user, FileInfo $file) {
}

public function rollback(IVersion $version) {
if (!$this->currentUserHasPermissions($version, \OCP\Constants::PERMISSION_UPDATE)) {
throw new Forbidden('You cannot restore this version because you do not have update permissions on the source file.');
}

return Storage::rollback($version->getVersionPath(), $version->getRevisionId(), $version->getUser());
}

Expand Down Expand Up @@ -219,6 +228,10 @@ public function getVersionFile(IUser $user, FileInfo $sourceFile, $revision): Fi
}

public function setVersionLabel(IVersion $version, string $label): void {
if (!$this->currentUserHasPermissions($version, \OCP\Constants::PERMISSION_UPDATE)) {
throw new Forbidden('You cannot label this version because you do not have update permissions on the source file.');
}

$versionEntity = $this->versionsMapper->findVersionForFileId(
$version->getSourceFile()->getId(),
$version->getTimestamp(),
Expand All @@ -231,6 +244,10 @@ public function setVersionLabel(IVersion $version, string $label): void {
}

public function deleteVersion(IVersion $version): void {
if (!$this->currentUserHasPermissions($version, \OCP\Constants::PERMISSION_DELETE)) {
throw new Forbidden('You cannot delete this version because you do not have delete permissions on the source file.');
}

Storage::deleteRevision($version->getVersionPath(), $version->getRevisionId());
$versionEntity = $this->versionsMapper->findVersionForFileId(
$version->getSourceFile()->getId(),
Expand Down Expand Up @@ -270,4 +287,23 @@ public function updateVersionEntity(File $sourceFile, int $revision, array $prop
public function deleteVersionsEntity(File $file): void {
$this->versionsMapper->deleteAllVersionsForFileId($file->getId());
}

private function currentUserHasPermissions(IVersion $version, int $permissions): bool {
$sourceFile = $version->getSourceFile();
$currentUserId = $this->userSession->getUser()?->getUID();

if ($currentUserId === null) {
throw new NotFoundException("No user logged in");
}

if ($sourceFile->getOwner()?->getUID() !== $currentUserId) {
$nodes = $this->rootFolder->getUserFolder($currentUserId)->getById($sourceFile->getId());
$sourceFile = array_pop($nodes);
if (!$sourceFile) {
throw new NotFoundException("Version file not accessible by current user");
}
}

return ($sourceFile->getPermissions() & $permissions) === $permissions;
}
}
39 changes: 35 additions & 4 deletions apps/files_versions/src/components/Version.vue
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
</div>
</template>
<template #actions>
<NcActionButton v-if="enableLabeling"
<NcActionButton v-if="enableLabeling && hasUpdatePermissions"
:close-after-click="true"
@click="labelUpdate">
<template #icon>
Expand All @@ -62,23 +62,24 @@
</template>
{{ t('files_versions', 'Compare to current version') }}
</NcActionButton>
<NcActionButton v-if="!isCurrent"
<NcActionButton v-if="!isCurrent && hasUpdatePermissions"
:close-after-click="true"
@click="restoreVersion">
<template #icon>
<BackupRestore :size="22" />
</template>
{{ t('files_versions', 'Restore version') }}
</NcActionButton>
<NcActionLink :href="downloadURL"
<NcActionLink v-if="isDownloadable"
:href="downloadURL"
:close-after-click="true"
:download="downloadURL">
<template #icon>
<Download :size="22" />
</template>
{{ t('files_versions', 'Download version') }}
</NcActionLink>
<NcActionButton v-if="!isCurrent && enableDeletion"
<NcActionButton v-if="!isCurrent && enableDeletion && hasDeletePermissions"
:close-after-click="true"
@click="deleteVersion">
<template #icon>
Expand Down Expand Up @@ -106,6 +107,9 @@ import { translate as t } from '@nextcloud/l10n'
import { joinPaths } from '@nextcloud/paths'
import { getRootUrl } from '@nextcloud/router'
import { loadState } from '@nextcloud/initial-state'
import { Permission } from '@nextcloud/files'
import { hasPermissions } from '../../../files_sharing/src/lib/SharePermissionsToolBox.js'
export default {
name: 'Version',
Expand Down Expand Up @@ -224,6 +228,33 @@ export default {
enableDeletion() {
return this.capabilities.files.version_deletion === true
},
/** @return {boolean} */
hasDeletePermissions() {
return hasPermissions(this.fileInfo.permissions, Permission.DELETE)
},
/** @return {boolean} */
hasUpdatePermissions() {
return hasPermissions(this.fileInfo.permissions, Permission.UPDATE)
},
/** @return {boolean} */
isDownloadable() {
if ((this.fileInfo.permissions & Permission.READ) === 0) {
return false
}
// If the mount type is a share, ensure it got download permissions.
if (this.fileInfo.mountType === 'shared') {
const downloadAttribute = this.fileInfo.shareAttributes.find((attribute) => attribute.scope === 'permissions' && attribute.key === 'download')
if (downloadAttribute !== undefined && downloadAttribute.enabled === false) {
return false
}
}
return true
},
},
methods: {
labelUpdate() {
Expand Down
4 changes: 2 additions & 2 deletions dist/files_versions-files_versions.js

Large diffs are not rendered by default.

22 changes: 22 additions & 0 deletions dist/files_versions-files_versions.js.LICENSE.txt
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,28 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

/**
* @copyright 2022 Louis Chmn <[email protected]>
*
* @author Louis Chmn <[email protected]>
*
* @license AGPL-3.0-or-later
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* 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
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

/**
* @copyright Copyright (c) 2021 John Molakvoæ <[email protected]>
*
Expand Down
2 changes: 1 addition & 1 deletion dist/files_versions-files_versions.js.map

Large diffs are not rendered by default.

0 comments on commit 7f33466

Please sign in to comment.