Skip to content

Commit

Permalink
fix(backend): Accept pushs with only step1 messages by read-only clients
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
mejo- committed Mar 12, 2024
1 parent 3033104 commit f55a15b
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 13 deletions.
15 changes: 4 additions & 11 deletions lib/Service/ApiService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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);
Expand Down
9 changes: 7 additions & 2 deletions lib/Service/DocumentService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 $token): array {
$documentId = $session->getDocumentId();
$stepsToInsert = [];
$querySteps = [];
Expand All @@ -224,6 +226,9 @@ public function addStep(Document $document, Session $session, array $steps, int
}
}
if (count($stepsToInsert) > 0) {
if ($this->isReadOnly($this->getFileForSession($session, $token), $token)) {
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
Expand Down

0 comments on commit f55a15b

Please sign in to comment.