From b0f673351b192e2eb7474764439c3da8250384ac Mon Sep 17 00:00:00 2001
From: Jonas <jonas@freesources.org>
Date: Tue, 12 Mar 2024 18:14:07 +0100
Subject: [PATCH] fix(backend): Accept pushs with only step1 messages by
 read-only clients

This fixes read-only clients getting 403 responses on push requests that
only contain questions for updates.

Fixes: #5223
Fixes: #5366
Fixes: #5368

Signed-off-by: Jonas <jonas@freesources.org>
---
 lib/Service/ApiService.php      | 15 ++++-----------
 lib/Service/DocumentService.php |  9 +++++++--
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/lib/Service/ApiService.php b/lib/Service/ApiService.php
index 288171640e0..436ddc73eee 100644
--- a/lib/Service/ApiService.php
+++ b/lib/Service/ApiService.php
@@ -184,11 +184,8 @@ public function close(int $documentId, int $sessionId, string $sessionToken): Da
 
 	/**
 	 * @throws NotFoundException
-	 * @throws DoesNotExistException
-	 *
-	 * @param null|string $token
 	 */
-	public function push(Session $session, Document $document, int $version, array $steps, string $awareness, string|null $token = null): DataResponse {
+	public function push(Session $session, Document $document, int $version, array $steps, string $awareness, ?string $token = null): DataResponse {
 		try {
 			$session = $this->sessionService->updateSessionAwareness($session, $awareness);
 		} catch (DoesNotExistException $e) {
@@ -198,16 +195,12 @@ public function push(Session $session, Document $document, int $version, array $
 		if (empty($steps)) {
 			return new DataResponse([]);
 		}
-		$file = $this->documentService->getFileForSession($session, $token);
-		if ($this->documentService->isReadOnly($file, $token)) {
-			return new DataResponse([], 403);
-		}
 		try {
-			$result = $this->documentService->addStep($document, $session, $steps, $version);
+			$result = $this->documentService->addStep($document, $session, $steps, $version, $token);
 		} catch (InvalidArgumentException $e) {
 			return new DataResponse($e->getMessage(), 422);
-		} catch (DoesNotExistException $e) {
-			// Session was removed in the meantime. #3875
+		} catch (DoesNotExistException|NotPermittedException) {
+			// Either no write access or session was removed in the meantime (#3875).
 			return new DataResponse([], 403);
 		}
 		return new DataResponse($result);
diff --git a/lib/Service/DocumentService.php b/lib/Service/DocumentService.php
index fa3c34e5b8d..1241f324588 100644
--- a/lib/Service/DocumentService.php
+++ b/lib/Service/DocumentService.php
@@ -206,10 +206,12 @@ public function writeDocumentState(int $documentId, string $content): void {
 	}
 
 	/**
-	 * @throws DoesNotExistException
 	 * @throws InvalidArgumentException
+	 * @throws NotFoundException
+	 * @throws NotPermittedException
+	 * @throws DoesNotExistException
 	 */
-	public function addStep(Document $document, Session $session, array $steps, int $version): array {
+	public function addStep(Document $document, Session $session, array $steps, int $version, ?string $shareToken): array {
 		$documentId = $session->getDocumentId();
 		$stepsToInsert = [];
 		$querySteps = [];
@@ -224,6 +226,9 @@ public function addStep(Document $document, Session $session, array $steps, int
 			}
 		}
 		if (count($stepsToInsert) > 0) {
+			if ($this->isReadOnly($this->getFileForSession($session, $shareToken), $shareToken)) {
+				throw new NotPermittedException('Read-only client tries to push steps with changes');
+			}
 			$newVersion = $this->insertSteps($document, $session, $stepsToInsert);
 		}
 		// If there were any queries in the steps send the entire history