Skip to content

Commit

Permalink
Block file access for secure view
Browse files Browse the repository at this point in the history
Signed-off-by: Julius Härtl <jus@bitgrid.net>
  • Loading branch information
juliusknorr committed Oct 17, 2022
1 parent e41db1a commit 7872390
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 7 deletions.
23 changes: 22 additions & 1 deletion lib/Service/ApiService.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

use Exception;
use OC\Files\Node\File;
use OCA\Files_Sharing\SharedStorage;
use OCA\Text\AppInfo\Application;
use OCA\Text\Exception\DocumentHasUnsavedChangesException;
use OCA\Text\Exception\DocumentSaveConflictException;
Expand All @@ -41,8 +42,11 @@
use OCP\Constants;
use OCP\Files\Lock\ILock;
use OCP\Files\NotFoundException;
use OCP\Files\NotPermittedException;
use OCP\IL10N;
use OCP\IRequest;
use OCP\Lock\LockedException;
use OCP\Share\IShare;
use Psr\Log\LoggerInterface;

class ApiService {
Expand All @@ -52,19 +56,23 @@ class ApiService {
private LoggerInterface $logger;
private AttachmentService $attachmentService;
private EncodingService $encodingService;
private IL10N $l10n;

public function __construct(IRequest $request,
SessionService $sessionService,
DocumentService $documentService,
AttachmentService $attachmentService,
EncodingService $encodingService,
LoggerInterface $logger) {
LoggerInterface $logger,
IL10N $l10n
) {
$this->request = $request;
$this->sessionService = $sessionService;
$this->documentService = $documentService;
$this->logger = $logger;
$this->attachmentService = $attachmentService;
$this->encodingService = $encodingService;
$this->l10n = $l10n;
}

public function create($fileId = null, $filePath = null, $token = null, $guestName = null, bool $forceRecreate = false): DataResponse {
Expand All @@ -81,13 +89,26 @@ public function create($fileId = null, $filePath = null, $token = null, $guestNa
$this->documentService->checkSharePermissions($token, Constants::PERMISSION_READ);
} catch (NotFoundException $e) {
return new DataResponse([], Http::STATUS_NOT_FOUND);
} catch (NotPermittedException $e) {
return new DataResponse($this->l10n->t('This file cannot be displayed as download is disabled by the share'), 404);
}
} elseif ($fileId) {
$file = $this->documentService->getFileById($fileId);
} else {
return new DataResponse('No valid file argument provided', 500);
}

$storage = $file->getStorage();

// Block using text for disabled download internal shares
if ($storage->instanceOfStorage(SharedStorage::class)) {
/** @var IShare $share */
$share = $storage->getShare();
if ($share->getAttributes()->getAttribute('permissions', 'download') === false) {
return new DataResponse($this->l10n->t('This file cannot be displayed as download is disabled by the share'), 403);
}
}

$readOnly = $this->documentService->isReadOnly($file, $token);

$this->sessionService->removeInactiveSessions($file->getId());
Expand Down
6 changes: 5 additions & 1 deletion lib/Service/DocumentService.php
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ public function getLockInfo($file): ?ILock {
/**
* @param $shareToken
* @return void
* @throws NotFoundException
* @throws NotFoundException|NotPermittedException
*/
public function checkSharePermissions($shareToken, $permission = Constants::PERMISSION_READ): void {
try {
Expand All @@ -494,6 +494,10 @@ public function checkSharePermissions($shareToken, $permission = Constants::PERM
if (($share->getPermissions() & $permission) === 0) {
throw new NotFoundException();
}

if ($share->getHideDownload()) {
throw new NotPermittedException();
}
}

public function hasUnsavedChanges(Document $document) {
Expand Down
20 changes: 17 additions & 3 deletions src/components/Editor.vue
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,11 @@
-->

<template>
<div data-text-el="editor-container" class="text-editor" @keydown.esc.stop.prevent="() => {}">
<DocumentStatus v-if="displayed"
<div id="editor-container"
data-text-el="editor-container"
class="text-editor"
@keydown.esc.stop.prevent="() => {}">
<DocumentStatus v-if="displayedStatus"
:idle="idle"
:lock="lock"
:sync-error="syncError"
Expand Down Expand Up @@ -266,6 +269,9 @@ export default {
displayed() {
return this.currentSession && this.active
},
displayedStatus() {
return this.displayed || !!this.syncError
},
renderMenus() {
return this.contentLoaded
&& this.isRichEditor
Expand Down Expand Up @@ -586,12 +592,19 @@ export default {
},

onError({ type, data }) {
this.$editor.setOptions({ editable: false })

this.$nextTick(() => {
this.$editor?.setEditable(false)
this.$emit('sync-service:error')
})

if (type === ERROR_TYPE.LOAD_ERROR) {
this.syncError = {
type,
data,
}
}

if (type === ERROR_TYPE.SAVE_COLLISSION && (!this.syncError || this.syncError.type !== ERROR_TYPE.SAVE_COLLISSION)) {
this.contentLoaded = true
this.syncError = {
Expand All @@ -610,6 +623,7 @@ export default {
if (type === ERROR_TYPE.SOURCE_NOT_FOUND) {
this.hasConnectionIssue = true
}

this.$emit('ready')
},

Expand Down
18 changes: 17 additions & 1 deletion src/components/Editor/DocumentStatus.vue
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,24 @@

<template>
<div class="document-status">
<p v-if="idle" class="msg">
<NcEmptyContent v-if="isLoadingError" :title="t('text', 'Failed to load file')" :description="syncError.data.data">
<template #icon>
<AlertOctagonOutline />
</template>
</NcEmptyContent>

<p v-else-if="idle" class="msg">
{{ t('text', 'Document idle for {timeout} minutes, click to continue editing', { timeout: IDLE_TIMEOUT }) }} <a class="button primary" @click="reconnect">{{ t('text', 'Reconnect') }}</a>
</p>

<p v-else-if="hasSyncCollission" class="msg icon-error">
{{ t('text', 'The document has been changed outside of the editor. The changes cannot be applied.') }}
</p>

<p v-else-if="hasConnectionIssue" class="msg">
{{ t('text', 'File could not be loaded. Please check your internet connection.') }} <a class="button primary" @click="reconnect">{{ t('text', 'Reconnect') }}</a>
</p>

<p v-if="lock" class="msg msg-locked">
<Lock /> {{ t('text', 'This file is opened read-only as it is currently locked by {user}.', { user: lock.displayName }) }}
</p>
Expand All @@ -40,13 +49,17 @@
<script>

import { ERROR_TYPE, IDLE_TIMEOUT } from './../../services/SyncService.js'
import AlertOctagonOutline from 'vue-material-design-icons/AlertOctagonOutline.vue'
import Lock from 'vue-material-design-icons/Lock.vue'
import { NcEmptyContent } from '@nextcloud/vue'

export default {
name: 'DocumentStatus',

components: {
AlertOctagonOutline,
Lock,
NcEmptyContent,
},

props: {
Expand Down Expand Up @@ -78,6 +91,9 @@ export default {
hasSyncCollission() {
return this.syncError && this.syncError.type === ERROR_TYPE.SAVE_COLLISSION
},
isLoadingError() {
return this.syncError && this.syncError.type === ERROR_TYPE.LOAD_ERROR
},
},

methods: {
Expand Down
2 changes: 1 addition & 1 deletion src/services/SyncService.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ class SyncService {
if (!error.response || error.code === 'ECONNABORTED') {
this.emit('error', { type: ERROR_TYPE.CONNECTION_FAILED, data: {} })
} else {
this.emit('error', { type: ERROR_TYPE.LOAD_ERROR, data: error.response.status })
this.emit('error', { type: ERROR_TYPE.LOAD_ERROR, data: error.response })
}
throw error
})
Expand Down
5 changes: 5 additions & 0 deletions tests/stub.phpstub
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,8 @@ namespace OC\User {
class NoUserException extends \Exception {}
}

namespace OCA\Files_Sharing {
abstract class SharedStorage implements \OCP\Files\Storage\IStorage {
abstract public function getShare(): \OCP\Share\IShare;
}
}

0 comments on commit 7872390

Please sign in to comment.