fixup! Mark non home storage as not encrypted

nextcloud · Aug 18, 2022 · 35b86ba · 35b86ba
1 parent e3db899
commit 35b86ba
Show file tree

Hide file tree

Showing 3 changed files with 79 additions and 32 deletions.
diff --git a/lib/Connector/Sabre/APlugin.php b/lib/Connector/Sabre/APlugin.php
@@ -109,14 +109,7 @@ protected function getFileNode(string $path): Node {
 			throw new Forbidden('No user session found');
 		}
 		$uid = $user->getUID();
-
-		try {
-			return $this->rootFolder
-				->getUserFolder($uid)
-				->get($path);
-		} catch (Exception $e) {
-			throw new NotFound('file not found', Http::STATUS_NOT_FOUND, $e);
-		}
+		return $this->pathCache->getFileNode($uid, $path, $this->rootFolder);
 	}
 
 	/**

diff --git a/lib/E2EEnabledPathCache.php b/lib/E2EEnabledPathCache.php
@@ -30,6 +30,7 @@
 use OCP\Files\IHomeStorage;
 use OCP\Files\Cache\ICache;
 use OCP\Cache\CappedMemoryCache;
+use Sabre\DAV\Exception\NotFound;
 
 class E2EEnabledPathCache {
 	/**
@@ -45,51 +46,87 @@ class E2EEnabledPathCache {
 
 	public function __construct() {
 		$this->perStorageEncryptedStateCache = new CappedMemoryCache();
+		$this->nodeCache = [];
 	}
 
 	/**
 	 * Checks if the path is an E2E folder or inside an E2E folder
 	 *
 	 * @param INode&Node $node
 	 */
-	public function isE2EEnabledPath($node, string $path): bool {
+	public function isE2EEnabledPath($node): bool {
 		$storage = $node->getStorage();
 		$cache = $storage->getCache();
-		return $this->getEncryptedStates($cache, $path, $storage);
+		return $this->getEncryptedStates($cache, $node, $storage);
+	}
+
+	/**
+	 * Get file system node of requested file
+	 * @throws NotFound
+	 */
+	public function getFileNode($uid, string $path, $rootFolder): Node {
+		if (!isset($this->nodeCache[$uid])) {
+			$this->nodeCache[$uid] = [];
+		} else if (isset($this->nodeCache[$uid][$path])) {
+			$node = $this->nodeCache[$uid][$path];
+			if ($node instanceof \Exception) {
+				throw new NotFound('file not found', Http::STATUS_NOT_FOUND, $node);
+			}
+			return $node;
+		}
+		try {
+			$node = $rootFolder
+				->getUserFolder($uid)
+				->get($path);
+			$this->nodeCache[$uid][$path] = $node;
+			return $node;
+		} catch (Exception $e) {
+			$this->nodeCache[$uid][$path] = $e;
+			throw new NotFound('file not found', Http::STATUS_NOT_FOUND, $e);
+		}
 	}
 
 	/**
 	 * Get the encryption state for the path
 	 */
-	protected function getEncryptedStates(ICache $cache, string $path, IStorage $storage): bool {
-		/** @psalm-suppress InvalidArgument */
+	protected function getEncryptedStates(ICache $cache, $node, IStorage $storage): bool {
 		if (!$storage->instanceOfStorage(IHomeStorage::class)) {
 			return false;
 		}
 
 		$storageId = $cache->getNumericStorageId();
-		if (isset($this->perStorageEncryptedStateCache[$storageId][$path])) {
-			return $this->perStorageEncryptedStateCache[$storageId][$path];
+		if (isset($this->perStorageEncryptedStateCache[$storageId][$node->getPath()])) {
+			return $this->perStorageEncryptedStateCache[$storageId][$node->getPath()];
 		}
 
 		$parentIds = [];
-		if ($path !== $this->dirname($path)) {
-			$cacheEntries = [];
-			$cacheEntry = $cache->get($path);
-			if ($cacheEntry !== false) {
-				if ($cacheEntry->isEncrypted()) {
-					// no need to go further down in the tree
-					$this->perStorageEncryptedStateCache[$storageId][$path] = true;
-					return true;
-				}
-			}
-			$isEncrypted = $this->getEncryptedStates($cache, $this->dirname($path), $storage);
-			$this->perStorageEncryptedStateCache[$storageId][$path] = $isEncrypted;
-			return $isEncrypted;
+		if ($node->getPath() === '/' || $node->getPath() === '') {
+			// root is never encrypted
+			$this->perStorageEncryptedStateCache[$storageId][$node->getPath()] = false;
+			return false;
+		}
+
+		if ($node->isEncrypted()) {
+			// no need to go further down in the tree
+			$this->perStorageEncryptedStateCache[$storageId][$node->getPath()] = true;
+			return true;
+		}
+
+		// go down more, but try first just with the parent path to spare a lot of
+		// queries if already cached
+		$parentPath = $this->dirname($node->getPath());
+		if (isset($this->perStorageEncryptedStateCache[$storageId][$parentPath])) {
+			return $this->perStorageEncryptedStateCache[$storageId][$parentPath];
+		}
+
+		if ($parentPath === '/' || $parentPath === '.') {
+			$this->perStorageEncryptedStateCache[$storageId][$node->getPath()] = false;
+			return false;
 		}
 
-		$this->perStorageEncryptedStateCache[$storageId][$path] = false;
-		return false;
+		$encrypted = $this->getEncryptedStates($cache, $node->getParent(), $storage);
+		$this->perStorageEncryptedStateCache[$storageId][$node->getPath()] = $encrypted;
+		return $encrypted;
 	}
 
 	protected function dirname(string $path): string {

diff --git a/tests/Unit/Connector/Sabre/LockPluginTest.php b/tests/Unit/Connector/Sabre/LockPluginTest.php
@@ -34,6 +34,8 @@
 use OCA\EndToEndEncryption\E2EEnabledPathCache;
 use OCP\Files\FileInfo;
 use OCP\Files\IRootFolder;
+use OCP\Files\Storage\IStorage;
+use OCP\Files\IHomeStorage;
 use OCP\Files\Node;
 use OCP\IUserSession;
 use Sabre\CalDAV\ICalendar;
@@ -598,17 +600,32 @@ public function testIsE2EEnabledPathEncryptedFolder():void {
 				$this->userSession,
 				$this->lockManager,
 				$this->userAgentManager,
-				$this->pathCache,
+				new E2EEnabledPathCache(),
 			])
 			->getMock();
 
+		$cache = $this->createMock(\OCP\Files\Storage\IStorage::class);
+		$cache->expects($this->any())
+			->method('getNumericStorageId')
+			->willReturn(42);
+
+		$storage = $this->createMock(\OCP\Files\Storage\IStorage::class);
+		$storage->expects($this->once())
+			->method('getCache')
+			->willReturn($cache);
+		$storage->expects($this->any())
+			->method('instanceOfStorage')
+			->with(IHomeStorage::class)
+			->willReturn(true);
+
 		$node = $this->createMock(Node::class);
 		$node->expects($this->once())
 			->method('isEncrypted')
 			->willReturn(true);
 		$node->expects($this->once())
-			->method('getType')
-			->willReturn(FileInfo::TYPE_FOLDER);
+			->method('getStorage')
+			->willReturn($storage);
+
 
 		$plugin->expects($this->once())
 			->method('getFileNode')