fix(Middleware): Response with 412 if baseVersionEtag doesn't match

Signed-off-by: Jonas <jonas@freesources.org>
nextcloud · Mar 20, 2024 · bee61f4 · bee61f4
1 parent 28d67a4
commit bee61f4
Show file tree

Hide file tree

Showing 11 changed files with 72 additions and 7 deletions.
diff --git 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
@@ -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/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;
@@ -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);

diff --git 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;
@@ -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
@@ -0,0 +1,9 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OCA\Text\Exception;
+
+class InvalidDocumentBaseVersionEtagException extends \Exception {
+
+}
diff --git a/lib/Middleware/Attribute/RequireDocumentBaseVersionEtag.php b/lib/Middleware/Attribute/RequireDocumentBaseVersionEtag.php
@@ -0,0 +1,9 @@
+<?php
+
+namespace OCA\Text\Middleware\Attribute;
+
+use Attribute;
+
+#[Attribute(Attribute::TARGET_METHOD)]
+class RequireDocumentBaseVersionEtag {
+}
diff --git a/lib/Middleware/SessionMiddleware.php b/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($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
@@ -116,7 +116,7 @@ public function create(?int $fileId = null, ?string $filePath = null, ?string $b
 			$document = $this->documentService->getDocument($file->getId());
 			$freshSession = $document === null;
 			if ($baseVersionEtag && $baseVersionEtag !== $document?->getBaseVersionEtag()) {
-				return new DataResponse($this->l10n->t('Editing session has expired. Please reload the page.'), Http::STATUS_CONFLICT);
+				return new DataResponse($this->l10n->t('Editing session has expired. Please reload the page.'), Http::STATUS_PRECONDITION_FAILED);
 			}
 
 			if ($freshSession) {
@@ -193,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($this->l10n->t('Editing session has expired. Please reload the page.'), Http::STATUS_PRECONDITION_FAILED);
 		}
 		if (empty($steps)) {
 			return new DataResponse([]);
@@ -204,7 +204,7 @@ public function push(Session $session, Document $document, int $version, array $
 			return new DataResponse($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($this->l10n->t('Editing session has expired. Please reload the page.'), Http::STATUS_PRECONDITION_FAILED);
 		}
 		return new DataResponse($result);
 	}

diff --git a/src/services/PollingBackend.js b/src/services/PollingBackend.js
@@ -170,6 +170,9 @@ class PollingBackend {
 					outsideChange: e.response.data.outsideChange,
 				},
 			})
+		} else if (e.response.status === 412) {
+			this.#syncService.emit('error', { type: ERROR_TYPE.LOAD_ERROR, data: e.response })
+			this.disconnect()
 		} else if (e.response.status === 403) {
 			this.#syncService.emit('error', { type: ERROR_TYPE.SOURCE_NOT_FOUND, data: {} })
 			this.disconnect()

diff --git a/src/services/SessionApi.js b/src/services/SessionApi.js
@@ -114,6 +114,7 @@ export class Connection {
 		return this.#post(this.#url(`session/${this.#document.id}/sync`), {
 			...this.#defaultParams,
 			filePath: this.#options.filePath,
+			baseVersionEtag: this.#document.baseVersionEtag,
 			version,
 		})
 	}
@@ -122,6 +123,7 @@ export class Connection {
 		return this.#post(this.#url(`session/${this.#document.id}/save`), {
 			...this.#defaultParams,
 			filePath: this.#options.filePath,
+			baseVersionEtag: this.#document.baseVersionEtag,
 			version,
 			autosaveContent,
 			documentState,
@@ -134,6 +136,7 @@ export class Connection {
 		return this.#post(this.#url(`session/${this.#document.id}/push`), {
 			...this.#defaultParams,
 			filePath: this.#options.filePath,
+			baseVersionEtag: this.#document.baseVersionEtag,
 			steps,
 			version,
 			awareness,

diff --git a/src/services/SyncService.js b/src/services/SyncService.js
@@ -187,7 +187,9 @@ class SyncService {
 				if (!response || code === 'ECONNABORTED') {
 					this.emit('error', { type: ERROR_TYPE.CONNECTION_FAILED, data: {} })
 				}
-				if (response?.status === 403) {
+				if (response?.status === 412) {
+					this.emit('error', { type: ERROR_TYPE.LOAD_ERROR, data: response })
+				} else if (response?.status === 403) {
 					if (!data.document) {
 						// either the session is invalid or the document is read only.
 						logger.error('failed to write to document - not allowed')