fix(encryption): Improve Update class and event listenening

to avoid back&forth between path and Node object Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
nextcloud · Feb 11, 2025 · e95bf85 · e95bf85
1 parent d5fcd9b
commit e95bf85
Show file tree

Hide file tree

Showing 6 changed files with 112 additions and 182 deletions.
diff --git a/lib/private/Encryption/EncryptionEventListener.php b/lib/private/Encryption/EncryptionEventListener.php
@@ -18,7 +18,6 @@
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\EventDispatcher\IEventListener;
 use OCP\Files\Events\Node\NodeRenamedEvent;
-use OCP\Files\Folder;
 use OCP\IUser;
 use OCP\IUserSession;
 use OCP\Share\Events\ShareCreatedEvent;
@@ -32,6 +31,7 @@ class EncryptionEventListener implements IEventListener {
 	public function __construct(
 		private IUserSession $userSession,
 		private SetupManager $setupManager,
+		private Manager $encryptionManager,
 	) {
 	}
 
@@ -43,17 +43,20 @@ public static function register(IEventDispatcher $dispatcher): void {
 	}
 
 	public function handle(Event $event): void {
+		if (!$this->encryptionManager->isEnabled()) {
+			return;
+		}
 		if ($event instanceof NodeRenamedEvent) {
-			$this->getUpdate()->postRename($event->getSource() instanceof Folder, $event->getSource()->getPath(), $event->getTarget()->getPath());
+			$this->getUpdate()->postRename($event->getSource(), $event->getTarget());
 		} elseif ($event instanceof ShareCreatedEvent) {
-			$this->getUpdate()->postShared($event->getShare()->getNodeType(), $event->getShare()->getNodeId());
+			$this->getUpdate()->postShared($event->getShare()->getNode());
 		} elseif ($event instanceof ShareDeletedEvent) {
 			// In case the unsharing happens in a background job, we don't have
 			// a session and we load instead the user from the UserManager
 			$owner = $event->getShare()->getNode()->getOwner();
-			$this->getUpdate($owner)->postUnshared($event->getShare()->getNodeType(), $event->getShare()->getNodeId());
+			$this->getUpdate($owner)->postUnshared($event->getShare()->getNode());
 		} elseif ($event instanceof NodeRestoredEvent) {
-			$this->getUpdate()->postRestore($event->getTarget() instanceof Folder, $event->getTarget()->getPath());
+			$this->getUpdate()->postRestore($event->getTarget());
 		}
 	}
 

diff --git a/lib/private/Encryption/EncryptionWrapper.php b/lib/private/Encryption/EncryptionWrapper.php
@@ -75,14 +75,6 @@ public function wrapStorage(string $mountPoint, IStorage $storage, IMountPoint $
 				\OC::$server->getGroupManager(),
 				\OC::$server->getConfig()
 			);
-			$update = new Update(
-				$util,
-				Filesystem::getMountManager(),
-				$this->manager,
-				$fileHelper,
-				$this->logger,
-				$uid
-			);
 			return new Encryption(
 				$parameters,
 				$this->manager,
@@ -91,7 +83,6 @@ public function wrapStorage(string $mountPoint, IStorage $storage, IMountPoint $
 				$fileHelper,
 				$uid,
 				$keyStorage,
-				$update,
 				$mountManager,
 				$this->arrayCache
 			);

diff --git a/lib/private/Encryption/Update.php b/lib/private/Encryption/Update.php
@@ -1,131 +1,85 @@
 <?php
 
+declare(strict_types=1);
+
 /**
  * SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
  * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
  * SPDX-License-Identifier: AGPL-3.0-only
  */
+
 namespace OC\Encryption;
 
 use InvalidArgumentException;
-use OC\Files\Filesystem;
-use OC\Files\Mount;
 use OC\Files\View;
 use OCP\Encryption\Exceptions\GenericEncryptionException;
+use OCP\Files\File as OCPFile;
+use OCP\Files\Folder;
+use OCP\Files\NotFoundException;
 use Psr\Log\LoggerInterface;
 
 /**
  * update encrypted files, e.g. because a file was shared
  */
 class Update {
-	/** @var Util */
-	protected $util;
-
-	/** @var \OC\Files\Mount\Manager */
-	protected $mountManager;
-
-	/** @var Manager */
-	protected $encryptionManager;
-
-	/** @var string */
-	protected $uid;
-
-	/** @var File */
-	protected $file;
-
-	/** @var LoggerInterface */
-	protected $logger;
-
-	/**
-	 * @param string $uid
-	 */
 	public function __construct(
-		Util $util,
-		Mount\Manager $mountManager,
-		Manager $encryptionManager,
-		File $file,
-		LoggerInterface $logger,
-		$uid,
+		protected Util $util,
+		protected Manager $encryptionManager,
+		protected File $file,
+		protected LoggerInterface $logger,
+		protected string $uid,
 	) {
-		$this->util = $util;
-		$this->mountManager = $mountManager;
-		$this->encryptionManager = $encryptionManager;
-		$this->file = $file;
-		$this->logger = $logger;
-		$this->uid = $uid;
 	}
 
 	/**
 	 * hook after file was shared
 	 */
-	public function postShared(string $nodeType, int $nodeId): void {
-		if ($this->encryptionManager->isEnabled()) {
-			if ($nodeType === 'file' || $nodeType === 'folder') {
-				$path = Filesystem::getPath($nodeId);
-				[$owner, $ownerPath] = $this->getOwnerPath($path);
-				$absPath = '/' . $owner . '/files/' . $ownerPath;
-				$this->update($nodeType === 'folder', $absPath);
-			}
-		}
+	public function postShared(OCPFile|Folder $node): void {
+		$this->update($node);
 	}
 
 	/**
 	 * hook after file was unshared
 	 */
-	public function postUnshared(string $nodeType, int $nodeId): void {
-		if ($this->encryptionManager->isEnabled()) {
-			if ($nodeType === 'file' || $nodeType === 'folder') {
-				$path = Filesystem::getPath($nodeId);
-				[$owner, $ownerPath] = $this->getOwnerPath($path);
-				$absPath = '/' . $owner . '/files/' . $ownerPath;
-				$this->update($nodeType === 'folder', $absPath);
-			}
-		}
+	public function postUnshared(OCPFile|Folder $node): void {
+		$this->update($node);
 	}
 
 	/**
 	 * inform encryption module that a file was restored from the trash bin,
 	 * e.g. to update the encryption keys
 	 */
-	public function postRestore(bool $directory, string $filePath): void {
-		if ($this->encryptionManager->isEnabled()) {
-			$path = Filesystem::normalizePath('/' . $this->uid . '/files/' . $filePath);
-			$this->update($directory, $path);
-		}
+	public function postRestore(OCPFile|Folder $node): void {
+		$this->update($node);
 	}
 
 	/**
 	 * inform encryption module that a file was renamed,
 	 * e.g. to update the encryption keys
 	 */
-	public function postRename(bool $directory, string $source, string $target): void {
-		if (
-			$this->encryptionManager->isEnabled() &&
-			dirname($source) !== dirname($target)
-		) {
-			[$owner, $ownerPath] = $this->getOwnerPath($target);
-			$absPath = '/' . $owner . '/files/' . $ownerPath;
-			$this->update($directory, $absPath);
+	public function postRename(OCPFile|Folder $source, OCPFile|Folder $target): void {
+		if (dirname($source->getPath()) !== dirname($target->getPath())) {
+			$this->update($target);
 		}
 	}
 
 	/**
-	 * get owner and path relative to data/<owner>/files
+	 * get owner and path relative to data/
 	 *
-	 * @param string $path path to file for current user
-	 * @return array ['owner' => $owner, 'path' => $path]
 	 * @throws \InvalidArgumentException
 	 */
-	protected function getOwnerPath($path) {
-		$info = Filesystem::getFileInfo($path);
-		$owner = Filesystem::getOwner($path);
+	protected function getOwnerPath(OCPFile|Folder $node): string {
+		$owner = $node->getOwner()?->getUID();
+		if ($owner === null) {
+			throw new InvalidArgumentException('No owner found for ' . $node->getId());
+		}
 		$view = new View('/' . $owner . '/files');
-		$path = $view->getPath($info->getId());
-		if ($path === null) {
-			throw new InvalidArgumentException('No file found for ' . $info->getId());
+		try {
+			$path = $view->getPath($node->getId());
+		} catch (NotFoundException $e) {
+			throw new InvalidArgumentException('No file found for ' . $node->getId(), previous:$e);
 		}
-
-		return [$owner, $path];
+		return '/' . $owner . '/files/' . $path;
 	}
 
 	/**
@@ -134,7 +88,7 @@ protected function getOwnerPath($path) {
 	 * @param string $path relative to data/
 	 * @throws Exceptions\ModuleDoesNotExistsException
 	 */
-	public function update(bool $directory, string $path): void {
+	public function update(OCPFile|Folder $node): void {
 		$encryptionModule = $this->encryptionManager->getEncryptionModule();
 
 		// if the encryption module doesn't encrypt the files on a per-user basis
@@ -143,15 +97,14 @@ public function update(bool $directory, string $path): void {
 			return;
 		}
 
+		$path = $this->getOwnerPath($node);
 		// if a folder was shared, get a list of all (sub-)folders
-		if ($directory) {
+		if ($node instanceof Folder) {
 			$allFiles = $this->util->getAllFiles($path);
 		} else {
 			$allFiles = [$path];
 		}
 
-
-
 		foreach ($allFiles as $file) {
 			$usersSharing = $this->file->getAccessList($file);
 			try {

diff --git a/lib/private/Files/Storage/Wrapper/Encryption.php b/lib/private/Files/Storage/Wrapper/Encryption.php
@@ -8,7 +8,6 @@
 namespace OC\Files\Storage\Wrapper;
 
 use OC\Encryption\Exceptions\ModuleDoesNotExistsException;
-use OC\Encryption\Update;
 use OC\Encryption\Util;
 use OC\Files\Cache\CacheEntry;
 use OC\Files\Filesystem;
@@ -49,7 +48,6 @@ public function __construct(
 		private IFile $fileHelper,
 		private ?string $uid,
 		private IStorage $keyStorage,
-		private Update $update,
 		private Manager $mountManager,
 		private ArrayCache $arrayCache,
 	) {