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 <[email protected]>
nextcloud · Mar 12, 2024 · 0e08405 · 0e08405
1 parent bfdee2a
commit 0e08405
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 13 deletions.
diff --git 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
@@ -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