Merge pull request #7933 from nextcloud/fix/attachments/validate-public-share-password

Improve share token handling in AttachmentService

Author: juliusknorr

lib/Service/AttachmentService.php

@@ -10,6 +10,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;
@@ -25,6 +26,7 @@
 use OCP\Files\SimpleFS\ISimpleFile;
 use OCP\FilesMetadata\IFilesMetadataManager;
 use OCP\IPreview;
+use OCP\ISession;
 use OCP\IURLGenerator;
 use OCP\Lock\LockedException;
 use OCP\Share\Exceptions\ShareNotFound;
@@ -41,6 +43,7 @@ public function __construct(
 		private IURLGenerator $urlGenerator,
 		private IFilenameValidator $filenameValidator,
 		private IFilesMetadataManager $filesMetadataManager,
+		private ISession $session,
 	) {
 	}
 
@@ -311,9 +314,33 @@ public function uploadAttachment(int $documentId, string $newFileName, $newFileR
 	 * @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);
@@ -429,25 +456,16 @@ public static function getUniqueFileName(Folder $dir, string $fileName): string
 
 	/**
 	 * 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

@@ -72,3 +72,9 @@ abstract class IVersion {
 		abstract public function getSourceFile(): \OCP\Files\File;
 	}
 }
+
+namespace OCA\DAV\Connector\Sabre {
+	class PublicAuth {
+		public const DAV_AUTHENTICATED = '';
+	}
+}