Merge pull request #5472 from nextcloud/backport/5466/stable27

[stable27] fix(backend): Accept pushs with only step1 messages by read-only clients
nextcloud · Mar 13, 2024 · 9dd63c9 · 9dd63c9
2 parents 6c2fa0a + ba7447f
commit 9dd63c9
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 10 deletions.
diff --git a/lib/Service/ApiService.php b/lib/Service/ApiService.php
@@ -184,7 +184,6 @@ public function close($documentId, $sessionId, $sessionToken): DataResponse {
 
 	/**
 	 * @throws NotFoundException
-	 * @throws DoesNotExistException
 	 */
 	public function push(Session $session, Document $document, $version, $steps, $awareness, $token = null): DataResponse {
 		try {
@@ -196,16 +195,12 @@ public function push(Session $session, Document $document, $version, $steps, $aw
 		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
@@ -210,11 +210,14 @@ public function writeDocumentState(int $documentId, string $content): void {
 	 * @param $sessionId
 	 * @param $steps
 	 * @param $version
+	 * @param $shareToken
 	 * @return array
-	 * @throws DoesNotExistException
 	 * @throws InvalidArgumentException
+	 * @throws NotFoundException
+	 * @throws NotPermittedException
+	 * @throws DoesNotExistException
 	 */
-	public function addStep(Document $document, Session $session, $steps, $version): array {
+	public function addStep(Document $document, Session $session, $steps, $version, $shareToken): array {
 		$sessionId = $session->getId();
 		$documentId = $session->getDocumentId();
 		$stepsToInsert = [];
@@ -233,6 +236,9 @@ public function addStep(Document $document, Session $session, $steps, $version):
 			}
 		}
 		if (sizeof($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, $version);
 		}
 		// If there were any queries in the steps send the entire history