Merge pull request #8751 from nextcloud/backport/8746/stable34

[stable34] Fix permission change in text doc

Author: max-nextcloud

lib/Service/ApiService.php

@@ -174,9 +174,11 @@ public function push(Session $session, Document $document, int $version, array $
 			$this->addToPushQueue($document, [$awareness, ...array_values($steps)]);
 		} catch (InvalidArgumentException $e) {
 			return new DataResponse(['error' => $e->getMessage()], Http::STATUS_UNPROCESSABLE_ENTITY);
-		} catch (DoesNotExistException|NotPermittedException) {
+		} catch (DoesNotExistException) {
 			// Either no write access or session was removed in the meantime (#3875).
 			return new DataResponse(['error' => $this->l10n->t('Editing session has expired. Please reload the page.')], Http::STATUS_PRECONDITION_FAILED);
+		} catch (NotPermittedException) {
+			return new DataResponse(['error' => $this->l10n->t('This document is read-only.')], Http::STATUS_FORBIDDEN);
 		}
 		return new DataResponse($result);
 	}
@@ -214,6 +216,7 @@ public function sync(Session $session, Document $document, int $version = 0, ?st
 
 			// ensure file is still present and accessible
 			$file = $this->documentService->getFileForSession($session, $shareToken);
+			$result['readOnly'] = $this->documentService->isReadOnly($file, $shareToken);
 			$this->documentService->assertNoOutsideConflict($document, $file);
 		} catch (NotPermittedException|NotFoundException|InvalidPathException $e) {
 			$this->logger->info($e->getMessage(), ['exception' => $e]);
@@ -261,6 +264,10 @@ public function save(Session $session, Document $document, int $version, string
 			} catch (LockedException) {
 				// Ignore locked exception since it might happen due to an autosave action happening at the same time
 			}
+		} catch (NotPermittedException) {
+			return new DataResponse([
+				'error' => $this->l10n->t('Read-only permission cannot save document changes. Please reload the page.')
+			], Http::STATUS_FORBIDDEN);
 		} catch (NotFoundException) {
 			return new DataResponse([], Http::STATUS_NOT_FOUND);
 		} catch (Exception $e) {

lib/Service/DocumentService.php

@@ -218,7 +218,8 @@ public function writeDocumentState(int $documentId, string $content): void {
 	 */
 	public function addStep(Document $document, Session $session, array $steps, int $version, ?int $recoveryAttempt, ?string $shareToken): array {
 		$documentId = $session->getDocumentId();
-		$readOnly = $this->isReadOnlyCached($session, $shareToken);
+		$file = $this->getFileForSession($session, $shareToken);
+		$readOnly = $this->isReadOnly($file, $shareToken);
 		$stepsToInsert = [];
 		$stepsIncludeQuery = false;
 		$documentState = null;
@@ -384,7 +385,7 @@ public function autosave(Document $document, File $file, int $version, string $a
 		$documentId = $document->getId();
 
 		if ($this->isReadOnly($file, $shareToken)) {
-			return $document;
+			throw new NotPermittedException('Read-only permission cannot save document changes. Please reload the page.');
 		}
 
 		$this->assertNoOutsideConflict($document, $file, $force);
@@ -600,29 +601,14 @@ public function getFileByShareToken(string $shareToken, ?string $path = null): F
 		throw new \InvalidArgumentException('No proper share data');
 	}
 
-	public function isReadOnlyCached(Session $session, ?string $shareToken = null): bool {
-		$cacheKey = 'read-only-' . $session->getId();
-		$isReadOnly = $this->cache->get($cacheKey);
-		if ($isReadOnly === null) {
-			$file = $this->getFileForSession($session, $shareToken);
-			$isReadOnly = $this->isReadOnly($file, $shareToken);
-			$this->cache->set($cacheKey, $isReadOnly, 60 * 5);
-			return $isReadOnly;
-		}
-
-		return $isReadOnly;
-	}
-
 	public function isReadOnly(File $file, ?string $token): bool {
-		$readOnly = true;
+		$readOnly = !$file->isUpdateable();
 		if ($token !== null) {
 			try {
 				$this->checkSharePermissions($token, Constants::PERMISSION_UPDATE);
-				$readOnly = false;
 			} catch (NotFoundException $e) {
+				$readOnly = true;
 			}
-		} else {
-			$readOnly = !$file->isUpdateable();
 		}
 
 		$lockInfo = $this->getLockInfo($file);

src/apis/sync.ts

@@ -56,6 +56,7 @@ interface SyncResponse {
 		awareness: Record<string, string>
 		document: Document
 		sessions: Session[]
+		readOnly?: boolean
 	}
 }
 

src/components/Editor.vue

@@ -97,6 +97,7 @@ import {
 } from './Editor.provider.ts'
 import ReadonlyBar from './Menu/ReadonlyBar.vue'
 
+import { showWarning } from '@nextcloud/dialogs'
 import { loadState } from '@nextcloud/initial-state'
 import { generateRemoteUrl } from '@nextcloud/router'
 import { Awareness } from 'y-protocols/awareness.js'
@@ -548,6 +549,7 @@ export default defineComponent({
 			bus.on('stateChange', this.onStateChange)
 			bus.on('idle', this.onIdle)
 			bus.on('save', this.onSave)
+			bus.on('permissionChange', this.onPermissionChange)
 		},
 
 		unlistenSyncServiceEvents() {
@@ -559,6 +561,7 @@ export default defineComponent({
 			bus.off('stateChange', this.onStateChange)
 			bus.off('idle', this.onIdle)
 			bus.off('save', this.onSave)
+			bus.off('permissionChange', this.onPermissionChange)
 		},
 
 		reconnect() {
@@ -693,7 +696,15 @@ export default defineComponent({
 			}
 
 			if (type === ERROR_TYPE.PUSH_FORBIDDEN) {
-				this.hasConnectionIssue = true
+				this.readOnly = true
+				this.editMode = false
+				this.setEditable(this.editMode)
+				showWarning(
+					t(
+						'text',
+						'Your editing permissions have been revoked. The document is now read-only.',
+					),
+				)
 				this.emit('push:forbidden')
 				return
 			}
@@ -747,6 +758,24 @@ export default defineComponent({
 			})
 		},
 
+		onPermissionChange({ readOnly }) {
+			this.readOnly = readOnly
+			this.editMode = !readOnly && !this.openReadOnlyEnabled
+			this.setEditable(this.editMode)
+			if (readOnly) {
+				showWarning(
+					t(
+						'text',
+						'Your editing permissions have been revoked. The document is now read-only.',
+					),
+				)
+			} else {
+				showWarning(
+					t('text', 'You now have edit permissions for this document.'),
+				)
+			}
+		},
+
 		onFocus() {
 			this.emit('focus')
 		},

src/services/PollingBackend.ts

@@ -60,6 +60,7 @@ interface PollData {
 	document: Document
 	sessions: Session[]
 	steps: Step[]
+	readOnly?: boolean
 }
 
 interface ConflictData extends PollData {
@@ -145,6 +146,16 @@ class PollingBackend {
 		const { document, sessions } = data
 		this.#fetchRetryCounter = 0
 
+		if (data.readOnly !== undefined && data.readOnly !== this.#readOnly) {
+			this.#readOnly = data.readOnly
+			this.#syncService.bus.emit('permissionChange', {
+				readOnly: this.#readOnly,
+			})
+			if (data.readOnly) {
+				this.maximumReadOnlyTimer()
+			}
+		}
+
 		this.#syncService.bus.emit('change', { document, sessions })
 		this.#syncService.receiveSteps(data)
 

src/services/SaveService.ts

@@ -3,13 +3,14 @@
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
 
+import { showError } from '@nextcloud/dialogs'
 import debounce from 'debounce'
 
 import type { ShallowRef } from 'vue'
 import { save, saveViaSendBeacon } from '../apis/save'
 import type { Connection } from '../composables/useConnection.ts'
 import { logger } from '../helpers/logger.js'
-import type { SyncService } from './SyncService'
+import { ERROR_TYPE, type SyncService } from './SyncService'
 
 /**
  * Interval to save the serialized document and the document state
@@ -74,6 +75,22 @@ class SaveService {
 			this.autosave.clear()
 		} catch (e) {
 			logger.error('Failed to save document.', { error: e })
+			const response = (
+				e as { response?: { status?: number; data?: { error?: string } } }
+			).response
+			if (response?.status === 403) {
+				// Document is now read-only; permissionChange from sync will update the UI
+				return
+			}
+			if (response?.status === 412) {
+				this.emit('error', {
+					type: ERROR_TYPE.LOAD_ERROR,
+					data: response,
+				})
+				if (response.data?.error) {
+					showError(response.data.error)
+				}
+			}
 			throw e
 		}
 	}

src/services/SyncService.ts

@@ -129,6 +129,9 @@ export declare type EventTypes = {
 
 	/* Emitted if the connection has been closed */
 	close: void
+
+	/* Emitted if the read only state of the document has changed */
+	permissionChange: { readOnly: boolean }
 }
 
 class SyncService {

tests/unit/Service/ApiServiceTest.php

@@ -3,6 +3,7 @@
 namespace OCA\Text\Tests;
 
 use OCA\Text\Db\Document;
+use OCA\Text\Db\Session;
 use OCA\Text\Service\ApiService;
 use OCA\Text\Service\ConfigService;
 use OCA\Text\Service\DocumentService;
@@ -70,6 +71,29 @@ public function testCreateNewSessionWithoutOwner() {
 		self::assertFalse($actual->getData()['hasOwner']);
 	}
 
+	public function testSaveWithNotPermittedException() {
+		$session = new Session();
+		$session->setDocumentId(123);
+
+		$document = new Document();
+
+		$file = $this->mockFile(123, 'admin');
+
+		$this->documentService->method('getFileForSession')->willReturn($file);
+		$this->documentService->method('autosave')->willThrowException(new  \OCP\Files\NotPermittedException());
+
+		$this->l10n->method('t')
+			->with('Read-only permission cannot save document changes. Please reload the page.')
+			->willReturn('Read-only permission cannot save document changes. Please reload the page.');
+
+		$response = $this->apiService->save($session, $document, 1, 'content', 'state');
+
+		self::assertEquals(\OCP\AppFramework\Http::STATUS_FORBIDDEN, $response->getStatus());
+		self::assertEquals('Read-only permission cannot save document changes. Please reload the page.',
+			$response->getData()['error']
+		);
+	}
+
 	private function mockFile(int $id, ?string $owner) {
 		$file = $this->createMock(\OCP\Files\File::class);
 		$storage = $this->createMock(\OCP\Files\Storage\IStorage::class);