Skip to content

[stable28] fix(ApiService): Catch NotPermittedException and return 404 #6022

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mejo- merged 2 commits into from
Jul 9, 2024
Merged

[stable28] fix(ApiService): Catch NotPermittedException and return 404 #6022

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b27f9d5
chore(ApiService): Use constants for http status codes everywhere
mejo- Jul 9, 2024
c952501
fix(ApiService): Catch NotPermittedException and return 404
mejo- Jul 9, 2024
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
32 changes: 17 additions & 15 deletions lib/Service/ApiService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,16 @@ public function create(?int $fileId = null, ?string $filePath = null, ?string $b
} catch (NotFoundException $e) {
return new DataResponse([], Http::STATUS_NOT_FOUND);
} catch (NotPermittedException $e) {
return new DataResponse(['error' => $this->l10n->t('This file cannot be displayed as download is disabled by the share')], 404);
return new DataResponse(['error' => $this->l10n->t('This file cannot be displayed as download is disabled by the share')], Http::STATUS_NOT_FOUND);
}
} elseif ($fileId) {
try {
$file = $this->documentService->getFileById($fileId);
} catch (NotFoundException|NotPermittedException $e) {
$this->logger->error('No permission to access this file', [ 'exception' => $e ]);
return new DataResponse(['error' => $this->l10n->t('No permission to access this file.')], Http::STATUS_NOT_FOUND);
return new DataResponse([
'error' => $this->l10n->t('File not found')
], Http::STATUS_NOT_FOUND);
}
} else {
return new DataResponse(['error' => 'No valid file argument provided'], Http::STATUS_PRECONDITION_FAILED);
Expand All @@ -106,7 +108,7 @@ public function create(?int $fileId = null, ?string $filePath = null, ?string $b
$share = $storage->getShare();
$shareAttribtues = $share->getAttributes();
if ($shareAttribtues !== null && $shareAttribtues->getAttribute('permissions', 'download') === false) {
return new DataResponse(['error' => $this->l10n->t('This file cannot be displayed as download is disabled by the share')], 403);
return new DataResponse(['error' => $this->l10n->t('This file cannot be displayed as download is disabled by the share')], Http::STATUS_FORBIDDEN);
}
}

Expand All @@ -132,7 +134,7 @@ public function create(?int $fileId = null, ?string $filePath = null, ?string $b
}
} catch (Exception $e) {
$this->logger->error($e->getMessage(), ['exception' => $e]);
return new DataResponse(['error' => 'Failed to create the document session'], 500);
return new DataResponse(['error' => 'Failed to create the document session'], Http::STATUS_INTERNAL_SERVER_ERROR);
}

/** @var Document $document */
Expand Down Expand Up @@ -207,7 +209,7 @@ public function push(Session $session, Document $document, int $version, array $
try {
$result = $this->documentService->addStep($document, $session, $steps, $version, $token);
} catch (InvalidArgumentException $e) {
return new DataResponse(['error' => $e->getMessage()], 422);
return new DataResponse(['error' => $e->getMessage()], Http::STATUS_UNPROCESSABLE_ENTITY);
} catch (DoesNotExistException|NotPermittedException) {
// Either no write access or session was removed in the meantime (#3875).
return new DataResponse(['error' => $this->l10n->t('Editing session has expired. Please reload the page.')], Http::STATUS_PRECONDITION_FAILED);
Expand All @@ -228,16 +230,16 @@ public function sync(Session $session, Document $document, int $version = 0, ?st
// ensure file is still present and accessible
$file = $this->documentService->getFileForSession($session, $shareToken);
$this->documentService->assertNoOutsideConflict($document, $file);
} catch (NotFoundException|InvalidPathException $e) {
} catch (NotPermittedException|NotFoundException|InvalidPathException $e) {
$this->logger->info($e->getMessage(), ['exception' => $e]);
return new DataResponse([
'message' => 'File not found'
], 404);
], Http::STATUS_NOT_FOUND);
} catch (DoesNotExistException $e) {
$this->logger->info($e->getMessage(), ['exception' => $e]);
return new DataResponse([
'message' => 'Document no longer exists'
], 404);
], Http::STATUS_NOT_FOUND);
} catch (DocumentSaveConflictException) {
try {
/** @psalm-suppress PossiblyUndefinedVariable */
Expand All @@ -247,22 +249,22 @@ public function sync(Session $session, Document $document, int $version = 0, ?st
}
}

return new DataResponse($result, isset($result['outsideChange']) ? 409 : 200);
return new DataResponse($result, isset($result['outsideChange']) ? Http::STATUS_CONFLICT : Http::STATUS_OK);
}

public function save(Session $session, Document $document, int $version = 0, ?string $autosaveContent = null, ?string $documentState = null, bool $force = false, bool $manualSave = false, ?string $shareToken = null): DataResponse {
try {
$file = $this->documentService->getFileForSession($session, $shareToken);
} catch (NotFoundException $e) {
} catch (NotPermittedException|NotFoundException $e) {
$this->logger->info($e->getMessage(), ['exception' => $e]);
return new DataResponse([
'message' => 'File not found'
], 404);
], Http::STATUS_NOT_FOUND);
} catch (DoesNotExistException $e) {
$this->logger->info($e->getMessage(), ['exception' => $e]);
return new DataResponse([
'message' => 'Document no longer exists'
], 404);
], Http::STATUS_NOT_FOUND);
}

$result = [];
Expand All @@ -275,15 +277,15 @@ public function save(Session $session, Document $document, int $version = 0, ?st
// Ignore locked exception since it might happen due to an autosave action happening at the same time
}
} catch (NotFoundException) {
return new DataResponse([], 404);
return new DataResponse([], Http::STATUS_NOT_FOUND);
} catch (Exception $e) {
$this->logger->error($e->getMessage(), ['exception' => $e]);
return new DataResponse([
'message' => 'Failed to autosave document'
], 500);
], Http::STATUS_INTERNAL_SERVER_ERROR);
}

return new DataResponse($result, isset($result['outsideChange']) ? 409 : 200);
return new DataResponse($result, isset($result['outsideChange']) ? Http::STATUS_CONFLICT : Http::STATUS_OK);
}

public function updateSession(Session $session, string $guestName): DataResponse {
Expand Down
1 change: 1 addition & 0 deletions lib/Service/DocumentService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,7 @@ public function getAll(): \Generator {
}

/**
* @throws NotPermittedException
* @throws NotFoundException
*/
public function getFileForSession(Session $session, ?string $shareToken = null): File {
Expand Down