[stable25] Block file access for secure view

cypress/e2e/share.spec.js

@@ -23,6 +23,7 @@
 
 import { randHash } from '../utils/index.js'
 const randUser = randHash()
+const recipient = randHash()
 
 describe('Open test.md in viewer', function() {
 	before(function() {
@@ -39,6 +40,8 @@ describe('Open test.md in viewer', function() {
 		cy.visit('/apps/files')
 		cy.get('.files-fileList tr[data-file="test.md"]')
 			.should('contain', 'test.md')
+
+		cy.nextcloudCreateUser(recipient, 'password')
 	})
 	beforeEach(function() {
 		cy.login(randUser, 'password')
@@ -121,4 +124,16 @@ describe('Open test.md in viewer', function() {
 			})
 	})
 
+	it('Share a file with download disabled shows an error', function() {
+		cy.shareFileToUser(randUser, 'password', 'test.md', recipient, {
+			attributes: '[{"scope":"permissions","key":"download","enabled":false}]',
+		}).then(() => {
+			cy.login(recipient, 'password')
+			cy.visit('/apps/files')
+			cy.openFile('test.md')
+			cy.getModal().find('.empty-content__title').should('contain', 'Failed to load file')
+			cy.getModal().getContent().should('not.exist')
+		})
+	})
+
 })

cypress/support/commands.js

@@ -141,7 +141,7 @@ Cypress.Commands.add('createFile', (target, content, mimeType = 'text/markdown')
 
 })
 
-Cypress.Commands.add('shareFileToUser', (userId, password, path, targetUserId) => {
+Cypress.Commands.add('shareFileToUser', (userId, password, path, targetUserId, shareData = {}) => {
 	cy.clearCookies()
 	cy.request({
 		method: 'POST',
@@ -151,6 +151,7 @@ Cypress.Commands.add('shareFileToUser', (userId, password, path, targetUserId) =
 			path,
 			shareType: 0,
 			shareWith: targetUserId,
+			...shareData,
 		},
 		auth: { user: userId, pass: password },
 		headers: {

lib/Service/ApiService.php

@@ -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;
@@ -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 {
@@ -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 {
@@ -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());

lib/Service/DocumentService.php

@@ -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 {
@@ -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) {

src/components/Editor.vue

@@ -22,7 +22,7 @@
 
 <template>
 	<div data-text-el="editor-container" class="text-editor">
-		<DocumentStatus v-if="displayed"
+		<DocumentStatus v-if="displayedStatus"
 			:idle="idle"
 			:lock="lock"
 			:sync-error="syncError"
@@ -279,6 +279,9 @@ export default {
 		displayed() {
 			return this.currentSession && this.active
 		},
+		displayedStatus() {
+			return this.displayed || !!this.syncError
+		},
 		renderRichEditorMenus() {
 			return this.contentLoaded
 				&& this.isRichEditor
@@ -604,12 +607,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 = {
@@ -628,6 +638,7 @@ export default {
 			if (type === ERROR_TYPE.SOURCE_NOT_FOUND) {
 				this.hasConnectionIssue = true
 			}
+
 			this.$emit('ready')
 		},
 

src/components/Editor/DocumentStatus.vue

@@ -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>
@@ -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: {
@@ -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: {

src/services/SyncService.js

@@ -118,7 +118,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
 			})

tests/stub.phpstub

@@ -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;
+	}
+}