Merge pull request #5523 from nextcloud/fix/baseversionetag_change

fix(sync): If `baseVersionEtag` changed, reset frontend

composer/composer/autoload_classmap.php

@@ -31,6 +31,7 @@
     'OCA\\Text\\Event\\LoadEditor' => $baseDir . '/../lib/Event/LoadEditor.php',
     'OCA\\Text\\Exception\\DocumentHasUnsavedChangesException' => $baseDir . '/../lib/Exception/DocumentHasUnsavedChangesException.php',
     'OCA\\Text\\Exception\\DocumentSaveConflictException' => $baseDir . '/../lib/Exception/DocumentSaveConflictException.php',
+    'OCA\\Text\\Exception\\InvalidDocumentBaseVersionEtagException' => $baseDir . '/../lib/Exception/InvalidDocumentBaseVersionEtagException.php',
     'OCA\\Text\\Exception\\InvalidSessionException' => $baseDir . '/../lib/Exception/InvalidSessionException.php',
     'OCA\\Text\\Exception\\UploadException' => $baseDir . '/../lib/Exception/UploadException.php',
     'OCA\\Text\\Exception\\VersionMismatchException' => $baseDir . '/../lib/Exception/VersionMismatchException.php',
@@ -45,6 +46,7 @@
     'OCA\\Text\\Listeners\\LoadViewerListener' => $baseDir . '/../lib/Listeners/LoadViewerListener.php',
     'OCA\\Text\\Listeners\\NodeCopiedListener' => $baseDir . '/../lib/Listeners/NodeCopiedListener.php',
     'OCA\\Text\\Listeners\\RegisterDirectEditorEventListener' => $baseDir . '/../lib/Listeners/RegisterDirectEditorEventListener.php',
+    'OCA\\Text\\Middleware\\Attribute\\RequireDocumentBaseVersionEtag' => $baseDir . '/../lib/Middleware/Attribute/RequireDocumentBaseVersionEtag.php',
     'OCA\\Text\\Middleware\\Attribute\\RequireDocumentSession' => $baseDir . '/../lib/Middleware/Attribute/RequireDocumentSession.php',
     'OCA\\Text\\Middleware\\Attribute\\RequireDocumentSessionOrUserOrShareToken' => $baseDir . '/../lib/Middleware/Attribute/RequireDocumentSessionOrUserOrShareToken.php',
     'OCA\\Text\\Middleware\\SessionMiddleware' => $baseDir . '/../lib/Middleware/SessionMiddleware.php',

composer/composer/autoload_static.php

@@ -46,6 +46,7 @@ class ComposerStaticInitText
         'OCA\\Text\\Event\\LoadEditor' => __DIR__ . '/..' . '/../lib/Event/LoadEditor.php',
         'OCA\\Text\\Exception\\DocumentHasUnsavedChangesException' => __DIR__ . '/..' . '/../lib/Exception/DocumentHasUnsavedChangesException.php',
         'OCA\\Text\\Exception\\DocumentSaveConflictException' => __DIR__ . '/..' . '/../lib/Exception/DocumentSaveConflictException.php',
+        'OCA\\Text\\Exception\\InvalidDocumentBaseVersionEtagException' => __DIR__ . '/..' . '/../lib/Exception/InvalidDocumentBaseVersionEtagException.php',
         'OCA\\Text\\Exception\\InvalidSessionException' => __DIR__ . '/..' . '/../lib/Exception/InvalidSessionException.php',
         'OCA\\Text\\Exception\\UploadException' => __DIR__ . '/..' . '/../lib/Exception/UploadException.php',
         'OCA\\Text\\Exception\\VersionMismatchException' => __DIR__ . '/..' . '/../lib/Exception/VersionMismatchException.php',
@@ -60,6 +61,7 @@ class ComposerStaticInitText
         'OCA\\Text\\Listeners\\LoadViewerListener' => __DIR__ . '/..' . '/../lib/Listeners/LoadViewerListener.php',
         'OCA\\Text\\Listeners\\NodeCopiedListener' => __DIR__ . '/..' . '/../lib/Listeners/NodeCopiedListener.php',
         'OCA\\Text\\Listeners\\RegisterDirectEditorEventListener' => __DIR__ . '/..' . '/../lib/Listeners/RegisterDirectEditorEventListener.php',
+        'OCA\\Text\\Middleware\\Attribute\\RequireDocumentBaseVersionEtag' => __DIR__ . '/..' . '/../lib/Middleware/Attribute/RequireDocumentBaseVersionEtag.php',
         'OCA\\Text\\Middleware\\Attribute\\RequireDocumentSession' => __DIR__ . '/..' . '/../lib/Middleware/Attribute/RequireDocumentSession.php',
         'OCA\\Text\\Middleware\\Attribute\\RequireDocumentSessionOrUserOrShareToken' => __DIR__ . '/..' . '/../lib/Middleware/Attribute/RequireDocumentSessionOrUserOrShareToken.php',
         'OCA\\Text\\Middleware\\SessionMiddleware' => __DIR__ . '/..' . '/../lib/Middleware/SessionMiddleware.php',

cypress/e2e/api/SessionApi.spec.js

@@ -344,6 +344,28 @@ describe('The session Api', function() {
 				.then(() => connection.close())
 		})
 
+		it('refuses create,push,sync,save with non-matching baseVersionEtag', function() {
+			cy.failToCreateTextSession(undefined, 'wrongBaseVersionEtag', { filePath: '', shareToken })
+				.its('status')
+				.should('eql', 412)
+
+			connection.setBaseVersionEtag('wrongBaseVersionEtag')
+
+			cy.failToPushSteps({ connection, steps: [messages.update], version })
+				.its('status')
+				.should('equal', 412)
+
+			cy.failToSyncSteps(connection, { version: 0 })
+				.its('status')
+				.should('equal', 412)
+
+			cy.failToSave(connection)
+				.its('status')
+				.should('equal', 412)
+
+			cy.then(() => connection.close())
+		})
+
 		it('recovers session even if last person leaves right after create', function() {
 			let joining
 			cy.log('Initial user pushes steps')

cypress/e2e/conflict.spec.js

@@ -54,7 +54,8 @@ variants.forEach(function({ fixture, mime }) {
 			cy.get('#viewer .modal-header button.header-close').click()
 			cy.get('#viewer').should('not.exist')
 			cy.openFile(fileName)
-			cy.get('.text-editor .document-status .icon-error')
+			cy.get('.text-editor .document-status')
+				.should('contain', 'Document has been changed outside of the editor.')
 			getWrapper()
 				.find('#read-only-editor')
 				.should('contain', 'Hello world')

cypress/e2e/share.spec.js

@@ -151,7 +151,7 @@ describe('Open test.md in viewer', function() {
 			cy.login(recipient)
 			cy.visit('/apps/files')
 			cy.openFile('test.md')
-			cy.getModal().find('.empty-content__name').should('contain', 'Failed to load file')
+			cy.getModal().find('.document-status').should('contain', 'This file cannot be displayed as download is disabled by the share')
 			cy.getModal().getContent().should('not.exist')
 		})
 	})

cypress/e2e/sync.spec.js

@@ -74,7 +74,7 @@ describe('Sync', () => {
 		}).as('sessionRequests')
 		cy.wait('@dead', { timeout: 30000 })
 		cy.get('#editor-container .document-status', { timeout: 30000 })
-			.should('contain', 'File could not be loaded')
+			.should('contain', 'Document could not be loaded.')
 			.then(() => {
 				reconnect = true
 			})
@@ -83,7 +83,7 @@ describe('Sync', () => {
 			.as('syncAfterRecovery')
 		cy.wait('@syncAfterRecovery', { timeout: 30000 })
 		cy.get('#editor-container .document-status', { timeout: 30000 })
-			.should('not.contain', 'File could not be loaded')
+			.should('not.contain', 'Document could not be loaded.')
 		// FIXME: There seems to be a bug where typed words maybe lost if not waiting for the new session
 		cy.wait('@syncAfterRecovery', { timeout: 10000 })
 		cy.getContent().type('* more content added after the lost connection{enter}')
@@ -109,12 +109,12 @@ describe('Sync', () => {
 
 		cy.wait('@sessionRequests', { timeout: 30000 })
 		cy.get('#editor-container .document-status', { timeout: 30000 })
-			.should('contain', 'File could not be loaded')
+			.should('contain', 'Document could not be loaded.')
 
 		cy.wait('@syncAfterRecovery', { timeout: 60000 })
 
 		cy.get('#editor-container .document-status', { timeout: 30000 })
-			.should('not.contain', 'File could not be loaded')
+			.should('not.contain', 'Document could not be loaded.')
 		// FIXME: There seems to be a bug where typed words maybe lost if not waiting for the new session
 		cy.wait('@syncAfterRecovery', { timeout: 10000 })
 		cy.getContent().type('* more content added after the lost connection{enter}')
@@ -126,6 +126,25 @@ describe('Sync', () => {
 			.should('include', 'after the lost connection')
 	})
 
+	it('shows warning when document session got cleaned up', () => {
+		cy.get('.save-status button')
+			.click()
+		cy.wait('@save')
+		cy.uploadTestFile('test.md')
+
+		cy.get('#editor-container .document-status', { timeout: 30000 })
+			.should('contain', 'Editing session has expired.')
+
+		// Reload button works
+		cy.get('#editor-container .document-status a.button')
+			.contains('Reload')
+			.click()
+
+		cy.getContent()
+		cy.get('#editor-container .document-status .notecard')
+			.should('not.exist')
+	})
+
 	it('passes the doc content from one session to the next', () => {
 		cy.closeFile()
 		cy.intercept({ method: 'PUT', url: '**/apps/text/session/*/create' })

cypress/support/sessions.js

@@ -36,30 +36,50 @@ Cypress.Commands.add('createTextSession', (fileId, options = {}) => {
 	return api.open({ fileId })
 })
 
-Cypress.Commands.add('failToCreateTextSession', (fileId) => {
-	const api = new SessionApi()
-	return api.open({ fileId })
+Cypress.Commands.add('failToCreateTextSession', (fileId, baseVersionEtag = null, options = {}) => {
+	const api = new SessionApi(options)
+	return api.open({ fileId, baseVersionEtag })
 		.then((response) => {
 			throw new Error('Expected request to fail - but it succeeded!')
-		})
-		.catch((err) => err.response)
+		}, (err) => err.response)
 })
 
 Cypress.Commands.add('pushSteps', ({ connection, steps, version, awareness = '' }) => {
 	return connection.push({ steps, version, awareness })
 		.then(response => response.data)
 })
 
+Cypress.Commands.add('failToPushSteps', ({ connection, steps, version, awareness = '' }) => {
+	return connection.push({ steps, version, awareness })
+		.then((response) => {
+			throw new Error('Expected request to fail - but it succeeded!')
+		}, (err) => err.response)
+})
+
 Cypress.Commands.add('syncSteps', (connection, options = { version: 0 }) => {
 	return connection.sync(options)
 		.then(response => response.data)
 })
 
+Cypress.Commands.add('failToSyncSteps', (connection, options = { version: 0 }) => {
+	return connection.sync(options)
+		.then((response) => {
+			throw new Error('Expected request to fail - but it succeeded!')
+		}, (err) => err.response)
+})
+
 Cypress.Commands.add('save', (connection, options = { version: 0 }) => {
 	return connection.save(options)
 		.then(response => response.data)
 })
 
+Cypress.Commands.add('failToSave', (connection, options = { version: 0 }) => {
+	return connection.save(options)
+		.then((response) => {
+			throw new Error('Expected request to fail - but it succeeded!')
+		}, (err) => err.response)
+})
+
 // Used to test for race conditions between the last push and the close request
 Cypress.Commands.add('pushAndClose', ({ connection, steps, version, awareness = '' }) => {
 	cy.log('Race between push and close')

lib/Controller/PublicSessionController.php

@@ -25,6 +25,7 @@
 
 namespace OCA\Text\Controller;
 
+use OCA\Text\Middleware\Attribute\RequireDocumentBaseVersionEtag;
 use OCA\Text\Middleware\Attribute\RequireDocumentSession;
 use OCA\Text\Service\ApiService;
 use OCP\AppFramework\Http\Attribute\NoAdminRequired;
@@ -80,8 +81,8 @@ protected function isPasswordProtected(): bool {
 
 	#[NoAdminRequired]
 	#[PublicPage]
-	public function create(string $token, ?string $file = null, ?string $guestName = null): DataResponse {
-		return $this->apiService->create(null, $file, $token, $guestName);
+	public function create(string $token, ?string $file = null, ?string $baseVersionEtag = null, ?string $guestName = null): DataResponse {
+		return $this->apiService->create(null, $file, $baseVersionEtag, $token, $guestName);
 	}
 
 	#[NoAdminRequired]
@@ -92,20 +93,23 @@ public function close(int $documentId, int $sessionId, string $sessionToken): Da
 
 	#[NoAdminRequired]
 	#[PublicPage]
+	#[RequireDocumentBaseVersionEtag]
 	#[RequireDocumentSession]
 	public function push(int $documentId, int $sessionId, string $sessionToken, int $version, array $steps, string $awareness, string $token): DataResponse {
 		return $this->apiService->push($this->getSession(), $this->getDocument(), $version, $steps, $awareness, $token);
 	}
 
 	#[NoAdminRequired]
 	#[PublicPage]
+	#[RequireDocumentBaseVersionEtag]
 	#[RequireDocumentSession]
 	public function sync(string $token, int $version = 0): DataResponse {
 		return $this->apiService->sync($this->getSession(), $this->getDocument(), $version, $token);
 	}
 
 	#[NoAdminRequired]
 	#[PublicPage]
+	#[RequireDocumentBaseVersionEtag]
 	#[RequireDocumentSession]
 	public function save(string $token, int $version = 0, ?string $autosaveContent = null, ?string $documentState = null, bool $force = false, bool $manualSave = false): DataResponse {
 		return $this->apiService->save($this->getSession(), $this->getDocument(), $version, $autosaveContent, $documentState, $force, $manualSave, $token);

lib/Controller/SessionController.php

@@ -25,6 +25,7 @@
 
 namespace OCA\Text\Controller;
 
+use OCA\Text\Middleware\Attribute\RequireDocumentBaseVersionEtag;
 use OCA\Text\Middleware\Attribute\RequireDocumentSession;
 use OCA\Text\Service\ApiService;
 use OCA\Text\Service\NotificationService;
@@ -57,8 +58,8 @@ public function __construct(
 	}
 
 	#[NoAdminRequired]
-	public function create(?int $fileId = null, ?string $file = null): DataResponse {
-		return $this->apiService->create($fileId, $file, null, null);
+	public function create(?int $fileId = null, ?string $file = null, ?string $baseVersionEtag = null): DataResponse {
+		return $this->apiService->create($fileId, $file, $baseVersionEtag, null, null);
 	}
 
 	#[NoAdminRequired]
@@ -69,6 +70,7 @@ public function close(int $documentId, int $sessionId, string $sessionToken): Da
 
 	#[NoAdminRequired]
 	#[PublicPage]
+	#[RequireDocumentBaseVersionEtag]
 	#[RequireDocumentSession]
 	public function push(int $version, array $steps, string $awareness): DataResponse {
 		try {
@@ -81,6 +83,7 @@ public function push(int $version, array $steps, string $awareness): DataRespons
 
 	#[NoAdminRequired]
 	#[PublicPage]
+	#[RequireDocumentBaseVersionEtag]
 	#[RequireDocumentSession]
 	public function sync(int $version = 0): DataResponse {
 		try {
@@ -93,6 +96,7 @@ public function sync(int $version = 0): DataResponse {
 
 	#[NoAdminRequired]
 	#[PublicPage]
+	#[RequireDocumentBaseVersionEtag]
 	#[RequireDocumentSession]
 	public function save(int $version = 0, ?string $autosaveContent = null, ?string $documentState = null, bool $force = false, bool $manualSave = false): DataResponse {
 		try {

lib/Exception/InvalidDocumentBaseVersionEtagException.php

@@ -0,0 +1,9 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OCA\Text\Exception;
+
+class InvalidDocumentBaseVersionEtagException extends \Exception {
+
+}

lib/Middleware/Attribute/RequireDocumentBaseVersionEtag.php

@@ -0,0 +1,9 @@
+<?php
+
+namespace OCA\Text\Middleware\Attribute;
+
+use Attribute;
+
+#[Attribute(Attribute::TARGET_METHOD)]
+class RequireDocumentBaseVersionEtag {
+}

lib/Middleware/SessionMiddleware.php

@@ -4,17 +4,21 @@
 
 use OC\User\NoUserException;
 use OCA\Text\Controller\ISessionAwareController;
+use OCA\Text\Exception\InvalidDocumentBaseVersionEtagException;
 use OCA\Text\Exception\InvalidSessionException;
+use OCA\Text\Middleware\Attribute\RequireDocumentBaseVersionEtag;
 use OCA\Text\Middleware\Attribute\RequireDocumentSession;
 use OCA\Text\Middleware\Attribute\RequireDocumentSessionOrUserOrShareToken;
 use OCA\Text\Service\DocumentService;
 use OCA\Text\Service\SessionService;
 use OCP\AppFramework\Controller;
-use OCP\AppFramework\Http\DataResponse;
+use OCP\AppFramework\Http;
+use OCP\AppFramework\Http\JSONResponse;
 use OCP\AppFramework\Http\Response;
 use OCP\AppFramework\Middleware;
 use OCP\Files\IRootFolder;
 use OCP\Files\NotPermittedException;
+use OCP\IL10N;
 use OCP\IRequest;
 use OCP\IUserSession;
 use OCP\Share\Exceptions\ShareNotFound;
@@ -30,11 +34,13 @@ public function __construct(
 		private IUserSession $userSession,
 		private IRootFolder $rootFolder,
 		private ShareManager $shareManager,
+		private IL10N $l10n,
 	) {
 	}
 
 	/**
 	 * @throws ReflectionException
+	 * @throws InvalidDocumentBaseVersionEtagException
 	 * @throws InvalidSessionException
 	 */
 	public function beforeController(Controller $controller, string $methodName): void {
@@ -52,11 +58,28 @@ public function beforeController(Controller $controller, string $methodName): vo
 			}
 		}
 
+		if (!empty($reflectionMethod->getAttributes(RequireDocumentBaseVersionEtag::class))) {
+			$this->assertDocumentBaseVersionEtag();
+		}
+
 		if (!empty($reflectionMethod->getAttributes(RequireDocumentSession::class))) {
 			$this->assertDocumentSession($controller);
 		}
 	}
 
+	/**
+	 * @throws InvalidDocumentBaseVersionEtagException
+	 */
+	private function assertDocumentBaseVersionEtag(): void {
+		$documentId = (int)$this->request->getParam('documentId');
+		$baseVersionEtag = $this->request->getParam('baseVersionEtag');
+
+		$document = $this->documentService->getDocument($documentId);
+		if ($baseVersionEtag && $document?->getBaseVersionEtag() !== $baseVersionEtag) {
+			throw new InvalidDocumentBaseVersionEtagException();
+		}
+	}
+
 	/**
 	 * @throws InvalidSessionException
 	 */
@@ -118,9 +141,13 @@ private function assertUserOrShareToken(ISessionAwareController $controller): vo
 		$controller->setDocument($document);
 	}
 
-	public function afterException($controller, $methodName, \Exception $exception): DataResponse|Response {
+	public function afterException($controller, $methodName, \Exception $exception): JSONResponse|Response {
+		if ($exception instanceof InvalidDocumentBaseVersionEtagException) {
+			return new JSONResponse(['error' => $this->l10n->t('Editing session has expired. Please reload the page.')], Http::STATUS_PRECONDITION_FAILED);
+		}
+
 		if ($exception instanceof InvalidSessionException) {
-			return new DataResponse([], 403);
+			return new JSONResponse([], 403);
 		}
 
 		return parent::afterException($controller, $methodName, $exception);