[stable28] Improve share token handling in AttachmentService

lib/Service/AttachmentService.php

@@ -27,6 +27,7 @@
 namespace OCA\Text\Service;
 
 use OC\User\NoUserException;
+use OCA\DAV\Connector\Sabre\PublicAuth;
 use OCA\Files_Sharing\SharedStorage;
 use OCA\Text\Controller\AttachmentController;
 use OCA\Text\Db\Session;
@@ -40,6 +41,7 @@
 use OCP\Files\NotPermittedException;
 use OCP\Files\SimpleFS\ISimpleFile;
 use OCP\IPreview;
+use OCP\ISession;
 use OCP\IURLGenerator;
 use OCP\Lock\LockedException;
 use OCP\Share\Exceptions\ShareNotFound;
@@ -52,7 +54,8 @@
 		private ShareManager $shareManager,
 		private IPreview $previewManager,
 		private IMimeTypeDetector $mimeTypeDetector,
-		private IURLGenerator $urlGenerator) {
+		private IURLGenerator $urlGenerator,
+		private ISession $session) {
 	}
 
 	/**
@@ -310,9 +313,33 @@
 	 * @throws NoUserException
 	 */
 	public function uploadAttachmentPublic(?int $documentId, string $newFileName, $newFileResource, string $shareToken): array {
-		if (!$this->hasUpdatePermissions($shareToken)) {
+		try {
+			$share = $this->shareManager->getShareByToken($shareToken);
+		} catch (ShareNotFound) {
+			throw new NotFoundException('Share not found');
+		}
+
+		if (!$this->hasUpdatePermissions($share)) {
 			throw new NotPermittedException('No write permissions');
 		}
+
+		if ($share->getPassword() !== null) {
+			$key = PublicAuth::DAV_AUTHENTICATED;
+
+			if (!$this->session->exists($key)) {
+				throw new NotPermittedException('Share not authenticated');
+			}
+
+			$allowedShareIds = $this->session->get($key);
+			if (!is_array($allowedShareIds)) {
+				throw new NotPermittedException('Share not authenticated');
+			}
+
+			if (!in_array($share->getId(), $allowedShareIds, true)) {
+				throw new NotPermittedException('Share not authenticated');
+			}
+		}
+
 		$textFile = $this->getTextFilePublic($documentId, $shareToken);
 		$saveDir = $this->getAttachmentDirectoryForFile($textFile, true);
 		$fileName = self::getUniqueFileName($saveDir, $newFileName);
@@ -398,25 +425,16 @@
 
 	/**
 	 * Check if the shared access has write permissions
-	 *
-	 * @param string $shareToken
-	 *
-	 * @return bool
 	 */
-	private function hasUpdatePermissions(string $shareToken): bool {
-		try {
-			$share = $this->shareManager->getShareByToken($shareToken);
-			return (
-				in_array(
-					$share->getShareType(),
-					[IShare::TYPE_LINK, IShare::TYPE_EMAIL, IShare::TYPE_ROOM],
-					true
-				)
-				&& $share->getPermissions() & Constants::PERMISSION_UPDATE
-				&& $share->getNode()->getPermissions() & Constants::PERMISSION_UPDATE);
-		} catch (ShareNotFound|NotFoundException $e) {
-			return false;
-		}
+	private function hasUpdatePermissions(IShare $share): bool {
+		return (
+			in_array(
+				$share->getShareType(),
+				[IShare::TYPE_LINK, IShare::TYPE_EMAIL, IShare::TYPE_ROOM],
+				true
+			)
+			&& $share->getPermissions() & Constants::PERMISSION_UPDATE
+			&& $share->getNode()->getPermissions() & Constants::PERMISSION_UPDATE);
 	}
 
 	/**

tests/stub.php

@@ -49,3 +49,9 @@ abstract public function setWantsNotification(bool $wantsNotification): void;
 		abstract public function setNotificationTarget(?string $notificationTarget): void;
 	}
 }
+
+namespace OCA\DAV\Connector\Sabre {
+	class PublicAuth {
+		public const DAV_AUTHENTICATED = '';
+	}
+}