Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix(Middleware): Response with 412 if baseVersionEtag doesn't match
Signed-off-by: Jonas <[email protected]>
  • Loading branch information
mejo- committed Mar 20, 2024
commit f61fb6f69dbdb177cc22dc6db4b63ffedc72c531
2 changes: 2 additions & 0 deletions composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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',
Expand Down
2 changes: 2 additions & 0 deletions composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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',
Expand Down
4 changes: 4 additions & 0 deletions lib/Controller/PublicSessionController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions lib/Controller/SessionController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
9 changes: 9 additions & 0 deletions lib/Exception/InvalidDocumentBaseVersionEtagException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?php

declare(strict_types=1);

namespace OCA\Text\Exception;

class InvalidDocumentBaseVersionEtagException extends \Exception {

}
9 changes: 9 additions & 0 deletions lib/Middleware/Attribute/RequireDocumentBaseVersionEtag.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?php

namespace OCA\Text\Middleware\Attribute;

use Attribute;

#[Attribute(Attribute::TARGET_METHOD)]
class RequireDocumentBaseVersionEtag {
}
33 changes: 30 additions & 3 deletions lib/Middleware/SessionMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand All @@ -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
*/
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions lib/Service/ApiService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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([]);
Expand All @@ -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);
}
Expand Down
3 changes: 3 additions & 0 deletions src/services/PollingBackend.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
3 changes: 3 additions & 0 deletions src/services/SessionApi.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
}
Expand All @@ -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,
Expand All @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion src/services/SyncService.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,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')
Expand Down