chore(cleanup): error handling

* `checkSharePermissions` has not been throwing `NotPermittedException` for a while.
  See #3765 .
* `InvalidArgumentException` was not being handled.
* throw `NotFoundException` if file cannot be found and there is no share token.

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

Author: max-nextcloud

lib/Context/FileContextFactory.php

@@ -62,16 +62,20 @@ public function buildForId(
 	}
 
 	/**
-	 * @throws NotPermittedException if not logged in
-	 * @throws NotFoundException if the file cannot be found
+	 * @throws NotFoundException if the share cannot be found or is not readable (file drop)
+	 * @throws \InvalidArgumentException if the share is not a file share and no file path is given
 	 */
 	public function buildForShareWithPath(
 		string $token,
 		?string $filePath,
 		?string $baseVersionEtag,
 	): FileContext {
 		$file = $this->fileService->getFileByShareToken($token, $filePath);
-		$this->fileService->checkSharePermissions($token, Constants::PERMISSION_READ);
+		/*
+		* Check if we have proper read access (files drop)
+		* If not then well 404 it is.
+		*/
+		$this->fileService->checkSharePermissions($token);
 		return $this->build($file, $baseVersionEtag, $token);
 	}
 

lib/Controller/PublicSessionController.php

@@ -19,7 +19,6 @@
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\PublicShareController;
 use OCP\Files\NotFoundException;
-use OCP\Files\NotPermittedException;
 use OCP\IL10N;
 use OCP\IRequest;
 use OCP\ISession;
@@ -74,17 +73,13 @@ protected function isPasswordProtected(): bool {
 	#[NoAdminRequired]
 	#[PublicPage]
 	public function create(string $token, ?string $filePath = null, ?string $baseVersionEtag = null, ?string $guestName = null): DataResponse {
-		/*
-			* Check if we have proper read access (files drop)
-			* If not then well 404 it is.
-			*/
 		try {
 			$context = $this->fileContextFactory->buildForShareWithPath($token, $filePath, $baseVersionEtag);
 			return $this->apiService->create($context, $guestName);
 		} catch (NotFoundException) {
 			return new DataResponse([], Http::STATUS_NOT_FOUND);
-		} catch (NotPermittedException) {
-			return new DataResponse(['error' => $this->l10n->t('This file cannot be displayed as download is disabled by the share')], Http::STATUS_NOT_FOUND);
+		} catch (\InvalidArgumentException) {
+			return new DataResponse([], Http::STATUS_UNPROCESSABLE_ENTITY);
 		}
 
 	}

lib/Service/FileService.php

@@ -43,7 +43,10 @@ public function getFileForSession(Session $session, ?string $shareToken = null):
 		if (!$session->isGuest()) {
 			try {
 				return $this->getFileById($session->getDocumentId(), $session->getUserId());
-			} catch (NotFoundException) {
+			} catch (NotFoundException $e) {
+				if ($shareToken === null) {
+					throw $e;
+				}
 				// We may still have a user session but on a public share link so move on
 			}
 		}
@@ -118,7 +121,8 @@ public function getFileById(int $fileId, string $userId): File {
 	}
 
 	/**
-	 * @throws NotFoundException
+	 * @throws NotFoundException if the share cannot be found based on the token
+	 * @throws \InvalidArgumentException if the share is not a File share and path is omitted.
 	 */
 	public function getFileByShareToken(string $shareToken, ?string $path = null): File {
 		try {
@@ -134,7 +138,7 @@ public function getFileByShareToken(string $shareToken, ?string $path = null): F
 		if ($node instanceof File) {
 			return $node;
 		}
-		throw new \InvalidArgumentException('No proper share data');
+		throw new \InvalidArgumentException('Invalid share data.');
 	}
 
 	public function isReadOnly(File $file, ?string $token): bool {
@@ -169,7 +173,7 @@ public function isDownloadDisabled(File $file): bool {
 	 *
 	 * @return void
 	 *
-	 * @throws NotFoundException|NotPermittedException
+	 * @throws NotFoundException
 	 *
 	 * @psalm-param 1|2 $permission
 	 */