diff --git a/composer/composer/autoload_classmap.php b/composer/composer/autoload_classmap.php index dcff792ea42..dd4f029eb45 100644 --- a/composer/composer/autoload_classmap.php +++ b/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', diff --git a/composer/composer/autoload_static.php b/composer/composer/autoload_static.php index a172981b0ef..aac06275a14 100644 --- a/composer/composer/autoload_static.php +++ b/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', diff --git a/cypress/e2e/api/SessionApi.spec.js b/cypress/e2e/api/SessionApi.spec.js index 6a102316a71..666737b1ebf 100644 --- a/cypress/e2e/api/SessionApi.spec.js +++ b/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') diff --git a/cypress/e2e/conflict.spec.js b/cypress/e2e/conflict.spec.js index 9e5021dc6c1..1396f6f9bef 100644 --- a/cypress/e2e/conflict.spec.js +++ b/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') diff --git a/cypress/e2e/share.spec.js b/cypress/e2e/share.spec.js index 5eb124baf8c..e8282d5b775 100644 --- a/cypress/e2e/share.spec.js +++ b/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') }) }) diff --git a/cypress/e2e/sync.spec.js b/cypress/e2e/sync.spec.js index 01a68ab7035..c6338d82804 100644 --- a/cypress/e2e/sync.spec.js +++ b/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' }) diff --git a/cypress/support/sessions.js b/cypress/support/sessions.js index 947185529e5..8f598887481 100644 --- a/cypress/support/sessions.js +++ b/cypress/support/sessions.js @@ -36,13 +36,12 @@ 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 = '' }) => { @@ -50,16 +49,37 @@ Cypress.Commands.add('pushSteps', ({ connection, 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') diff --git a/lib/Controller/PublicSessionController.php b/lib/Controller/PublicSessionController.php index 3d0da080e8c..9d6177584ed 100644 --- a/lib/Controller/PublicSessionController.php +++ b/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,6 +93,7 @@ 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); @@ -99,6 +101,7 @@ public function push(int $documentId, int $sessionId, string $sessionToken, int #[NoAdminRequired] #[PublicPage] + #[RequireDocumentBaseVersionEtag] #[RequireDocumentSession] public function sync(string $token, int $version = 0): DataResponse { return $this->apiService->sync($this->getSession(), $this->getDocument(), $version, $token); @@ -106,6 +109,7 @@ public function sync(string $token, int $version = 0): DataResponse { #[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); diff --git a/lib/Controller/SessionController.php b/lib/Controller/SessionController.php index aa935f57879..19b241d0ce5 100644 --- a/lib/Controller/SessionController.php +++ b/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 { diff --git a/lib/Exception/InvalidDocumentBaseVersionEtagException.php b/lib/Exception/InvalidDocumentBaseVersionEtagException.php new file mode 100644 index 00000000000..1bb13ca65a2 --- /dev/null +++ b/lib/Exception/InvalidDocumentBaseVersionEtagException.php @@ -0,0 +1,9 @@ +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); diff --git a/lib/Service/ApiService.php b/lib/Service/ApiService.php index 9d3fd6410e3..488b6986db0 100644 --- a/lib/Service/ApiService.php +++ b/lib/Service/ApiService.php @@ -71,7 +71,7 @@ public function __construct(IRequest $request, $this->l10n = $l10n; } - public function create(?int $fileId = null, ?string $filePath = null, ?string $token = null, ?string $guestName = null): DataResponse { + public function create(?int $fileId = null, ?string $filePath = null, ?string $baseVersionEtag = null, ?string $token = null, ?string $guestName = null): DataResponse { try { if ($token) { $file = $this->documentService->getFileByShareToken($token, $this->request->getParam('filePath')); @@ -85,17 +85,17 @@ public function create(?int $fileId = null, ?string $filePath = null, ?string $t } 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); + return new DataResponse(['error' => $this->l10n->t('This file cannot be displayed as download is disabled by the share')], 404); } } elseif ($fileId) { try { $file = $this->documentService->getFileById($fileId); } catch (NotFoundException|NotPermittedException $e) { $this->logger->error('No permission to access this file', [ 'exception' => $e ]); - return new DataResponse($this->l10n->t('No permission to access this file.'), Http::STATUS_NOT_FOUND); + return new DataResponse(['error' => $this->l10n->t('No permission to access this file.')], Http::STATUS_NOT_FOUND); } } else { - return new DataResponse('No valid file argument provided', Http::STATUS_PRECONDITION_FAILED); + return new DataResponse(['error' => 'No valid file argument provided'], Http::STATUS_PRECONDITION_FAILED); } $storage = $file->getStorage(); @@ -106,7 +106,7 @@ public function create(?int $fileId = null, ?string $filePath = null, ?string $t $share = $storage->getShare(); $shareAttribtues = $share->getAttributes(); if ($shareAttribtues !== null && $shareAttribtues->getAttribute('permissions', 'download') === false) { - return new DataResponse($this->l10n->t('This file cannot be displayed as download is disabled by the share'), 403); + return new DataResponse(['error' => $this->l10n->t('This file cannot be displayed as download is disabled by the share')], 403); } } @@ -115,6 +115,9 @@ public function create(?int $fileId = null, ?string $filePath = null, ?string $t $this->sessionService->removeInactiveSessionsWithoutSteps($file->getId()); $document = $this->documentService->getDocument($file->getId()); $freshSession = $document === null; + if ($baseVersionEtag && $baseVersionEtag !== $document?->getBaseVersionEtag()) { + return new DataResponse(['error' => $this->l10n->t('Editing session has expired. Please reload the page.')], Http::STATUS_PRECONDITION_FAILED); + } if ($freshSession) { $this->logger->info('Create new document of ' . $file->getId()); @@ -129,7 +132,7 @@ public function create(?int $fileId = null, ?string $filePath = null, ?string $t } } catch (Exception $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); - return new DataResponse('Failed to create the document session', 500); + return new DataResponse(['error' => 'Failed to create the document session'], 500); } /** @var Document $document */ @@ -190,7 +193,7 @@ public function push(Session $session, Document $document, int $version, array $ $session = $this->sessionService->updateSessionAwareness($session, $awareness); } catch (DoesNotExistException $e) { // Session was removed in the meantime. #3875 - return new DataResponse([], 403); + return new DataResponse(['error' => $this->l10n->t('Editing session has expired. Please reload the page.')], Http::STATUS_PRECONDITION_FAILED); } if (empty($steps)) { return new DataResponse([]); @@ -198,10 +201,10 @@ public function push(Session $session, Document $document, int $version, array $ try { $result = $this->documentService->addStep($document, $session, $steps, $version, $token); } catch (InvalidArgumentException $e) { - return new DataResponse($e->getMessage(), 422); + return new DataResponse(['error' => $e->getMessage()], 422); } catch (DoesNotExistException|NotPermittedException) { // Either no write access or session was removed in the meantime (#3875). - return new DataResponse([], 403); + return new DataResponse(['error' => $this->l10n->t('Editing session has expired. Please reload the page.')], Http::STATUS_PRECONDITION_FAILED); } return new DataResponse($result); } diff --git a/src/components/Editor.vue b/src/components/Editor.vue index 6378aded67d..ddd0b122359 100644 --- a/src/components/Editor.vue +++ b/src/components/Editor.vue @@ -374,6 +374,7 @@ export default { guestName, shareToken: this.shareToken, filePath: this.relativePath, + baseVersionEtag: this.$syncService?.baseVersionEtag, forceRecreate: this.forceRecreate, serialize: this.isRichEditor ? (content) => createMarkdownSerializer(this.$editor.schema).serialize(content ?? this.$editor.state.doc) diff --git a/src/components/Editor/DocumentStatus.vue b/src/components/Editor/DocumentStatus.vue index 3782b765197..0061099826c 100644 --- a/src/components/Editor/DocumentStatus.vue +++ b/src/components/Editor/DocumentStatus.vue @@ -22,27 +22,34 @@