fix(api): Respond to missing file with 404

Use exceptions to destinguish between not found and forbidden.

We try to not expose wether a file exists or not.
But if the file exists but download permissions have not been granted
we want to be explicit about that and return a 403 response code.

Signed-off-by: Max <max@nextcloud.com>

Author: max-nextcloud

cypress/e2e/api/SessionApi.spec.js

@@ -54,7 +54,7 @@ describe('The session Api', function() {
 		})
 
 		it('handles missing file id', function() {
-			cy.failToCreateTextSession().its('status').should('equal', 412)
+			cy.failToCreateTextSession().its('status').should('equal', 404)
 		})
 	})
 

lib/Context/FileContext.php

@@ -57,6 +57,10 @@ public function toString(): string {
 		return $this->getType() . ' (' . $this->getId() . ')';
 	}
 
+	/**
+	 * @throws NotFoundException
+	 * @throws NotPermittedException
+	 */
 	#[Override]
 	public function getFile(): File {
 		if ($this->file !== null) {
@@ -73,12 +77,23 @@ public function getFile(): File {
 		return $this->file;
 	}
 
+	/**
+	 * @throws NotFoundException
+	 * @throws NotPermittedException
+	 */
 	#[Override]
-	public function buildDocument(): Document|string {
-		$file = $this->getFile();
+	public function buildDocument(): Document {
+		try {
+			$file = $this->getFile();
+		} catch (NotFoundException|NotPermittedException $e) {
+			$this->logger->warning('No permission to access this file', ['exception' => $e]);
+			throw new NotFoundException($this->l10n->t('File not found'), $e->getCode(), $e);
+		}
 		// Block using text for disabled download internal shares
 		if ($this->fileService->isDownloadDisabled($file)) {
-			return $this->l10n->t('This file cannot be displayed as download is disabled by the share');
+			throw new NotPermittedException(
+				$this->l10n->t('This file cannot be displayed as download is disabled by the share')
+			);
 		}
 		$document = new Document();
 		$document->setContextType('file');

lib/Context/IContext.php

@@ -11,15 +11,25 @@
 use OCA\Text\Db\Session;
 use OCP\Files\File;
 use OCP\Files\Lock\ILock;
+use OCP\Files\NotFoundException;
+use OCP\Files\NotPermittedException;
 
 interface IContext {
 	public function getId(): int;
 	public function getType(): string;
 	public function toString(): string;
-	public function buildDocument(): Document|string;
+	/**
+	 * @throws NotFoundException
+	 * @throws NotPermittedException
+	 */
+	public function buildDocument(): Document;
 	public function prepareSession(DocumentData $documentData): SessionInfo;
 	public function isReadOnly(): bool;
 	public function updateDocument(Document $document): ?Document;
+	/**
+	 * @throws NotFoundException
+	 * @throws NotPermittedException
+	 */
 	public function getFile(): ?File;
 	public function loadContent(): ?string;
 	public function saveWithLock(string $content, callable $doWhileLocked): void;

lib/Service/ApiService.php

@@ -21,7 +21,6 @@
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\DataResponse;
-use OCP\Files\File;
 use OCP\Files\InvalidPathException;
 use OCP\Files\NotFoundException;
 use OCP\Files\NotPermittedException;
@@ -37,16 +36,18 @@ public function __construct(
 		private readonly SessionService $sessionService,
 		private readonly DocumentService $documentService,
 		private readonly LoggerInterface $logger,
-		private readonly LockService $lockService,
 		private readonly IL10N $l10n,
 		private readonly ?IQueue $queue,
 	) {
 	}
 
 	public function create(IContext $context, ?string $baseVersionEtag, ?string $guestName = null): DataResponse {
-		$document = $context->buildDocument();
-		if (!$document instanceof Document) {
-			return new DataResponse(['error' => $document], Http::STATUS_FORBIDDEN);
+		try {
+			$document = $context->buildDocument();
+		} catch (NotFoundException $e) {
+			return new DataResponse(['error' => $e->getMessage()], Http::STATUS_NOT_FOUND);
+		} catch (NotPermittedException $e) {
+			return new DataResponse(['error' => $e->getMessage()], Http::STATUS_FORBIDDEN);
 		}
 
 		try {