feat(encryption): Migrate from hooks to events

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>

apps/files_versions/lib/Listener/FileEventsListener.php

@@ -250,7 +250,7 @@ class FileEventsListener implements IEventListener {
 	/**
 	 * Erase versions of deleted file
 	 *
-	 * This function is connected to the delete signal of OC_Filesystem
+	 * This function is connected to the NodeDeletedEvent event
 	 * cleanup the versions directory if the actual file gets deleted
 	 */
 	public function remove_hook(Node $node): void {
@@ -282,7 +282,7 @@ class FileEventsListener implements IEventListener {
 	/**
 	 * rename/move versions of renamed/moved files
 	 *
-	 * This function is connected to the rename signal of OC_Filesystem and adjust the name and location
+	 * This function is connected to the NodeRenamedEvent event and adjust the name and location
 	 * of the stored versions along the actual file
 	 */
 	public function rename_hook(Node $source, Node $target): void {
@@ -301,7 +301,7 @@ class FileEventsListener implements IEventListener {
 	/**
 	 * copy versions of copied files
 	 *
-	 * This function is connected to the copy signal of OC_Filesystem and copies the
+	 * This function is connected to the NodeCopiedEvent event and copies the
 	 * the stored versions to the new location
 	 */
 	public function copy_hook(Node $source, Node $target): void {

lib/base.php

@@ -6,13 +6,14 @@ declare(strict_types=1);
  * SPDX-FileCopyrightText: 2013-2016 ownCloud, Inc.
  * SPDX-License-Identifier: AGPL-3.0-only
  */
-use OC\Encryption\HookManager;
+
 use OC\Profiler\BuiltInProfiler;
 use OC\Share20\GroupDeletedListener;
 use OC\Share20\Hooks;
 use OC\Share20\UserDeletedListener;
 use OC\Share20\UserRemovedListener;
 use OCP\EventDispatcher\IEventDispatcher;
+use OCP\Files\Events\BeforeFileSystemSetupEvent;
 use OCP\Group\Events\GroupDeletedEvent;
 use OCP\Group\Events\UserRemovedEvent;
 use OCP\IConfig;
@@ -22,7 +23,6 @@ use OCP\IURLGenerator;
 use OCP\IUserSession;
 use OCP\Security\Bruteforce\IThrottler;
 use OCP\Server;
-use OCP\Share;
 use OCP\Template\ITemplateManager;
 use OCP\User\Events\UserChangedEvent;
 use OCP\User\Events\UserDeletedEvent;
@@ -907,15 +907,16 @@ class OC {
 	}
 
 	private static function registerEncryptionWrapperAndHooks(): void {
+		/** @var \OC\Encryption\Manager */
 		$manager = Server::get(\OCP\Encryption\IManager::class);
-		\OCP\Util::connectHook('OC_Filesystem', 'preSetup', $manager, 'setupStorage');
+		Server::get(IEventDispatcher::class)->addListener(
+			BeforeFileSystemSetupEvent::class,
+			$manager->setupStorage(...),
+		);
 
 		$enabled = $manager->isEnabled();
 		if ($enabled) {
-			\OCP\Util::connectHook(Share::class, 'post_shared', HookManager::class, 'postShared');
-			\OCP\Util::connectHook(Share::class, 'post_unshare', HookManager::class, 'postUnshared');
-			\OCP\Util::connectHook('OC_Filesystem', 'post_rename', HookManager::class, 'postRename');
-			\OCP\Util::connectHook('\OCA\Files_Trashbin\Trashbin', 'post_restore', HookManager::class, 'postRestore');
+			\OC\Encryption\EncryptionEventListener::register(Server::get(IEventDispatcher::class));
 		}
 	}
 

lib/composer/composer/autoload_classmap.php

@@ -1539,6 +1539,7 @@ return array(
     'OC\\DirectEditing\\Token' => $baseDir . '/lib/private/DirectEditing/Token.php',
     'OC\\EmojiHelper' => $baseDir . '/lib/private/EmojiHelper.php',
     'OC\\Encryption\\DecryptAll' => $baseDir . '/lib/private/Encryption/DecryptAll.php',
+    'OC\\Encryption\\EncryptionEventListener' => $baseDir . '/lib/private/Encryption/EncryptionEventListener.php',
     'OC\\Encryption\\EncryptionWrapper' => $baseDir . '/lib/private/Encryption/EncryptionWrapper.php',
     'OC\\Encryption\\Exceptions\\DecryptionFailedException' => $baseDir . '/lib/private/Encryption/Exceptions/DecryptionFailedException.php',
     'OC\\Encryption\\Exceptions\\EmptyEncryptionDataException' => $baseDir . '/lib/private/Encryption/Exceptions/EmptyEncryptionDataException.php',
@@ -1549,7 +1550,6 @@ return array(
     'OC\\Encryption\\Exceptions\\ModuleDoesNotExistsException' => $baseDir . '/lib/private/Encryption/Exceptions/ModuleDoesNotExistsException.php',
     'OC\\Encryption\\Exceptions\\UnknownCipherException' => $baseDir . '/lib/private/Encryption/Exceptions/UnknownCipherException.php',
     'OC\\Encryption\\File' => $baseDir . '/lib/private/Encryption/File.php',
-    'OC\\Encryption\\HookManager' => $baseDir . '/lib/private/Encryption/HookManager.php',
     'OC\\Encryption\\Keys\\Storage' => $baseDir . '/lib/private/Encryption/Keys/Storage.php',
     'OC\\Encryption\\Manager' => $baseDir . '/lib/private/Encryption/Manager.php',
     'OC\\Encryption\\Update' => $baseDir . '/lib/private/Encryption/Update.php',

lib/composer/composer/autoload_static.php

@@ -1580,6 +1580,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OC\\DirectEditing\\Token' => __DIR__ . '/../../..' . '/lib/private/DirectEditing/Token.php',
         'OC\\EmojiHelper' => __DIR__ . '/../../..' . '/lib/private/EmojiHelper.php',
         'OC\\Encryption\\DecryptAll' => __DIR__ . '/../../..' . '/lib/private/Encryption/DecryptAll.php',
+        'OC\\Encryption\\EncryptionEventListener' => __DIR__ . '/../../..' . '/lib/private/Encryption/EncryptionEventListener.php',
         'OC\\Encryption\\EncryptionWrapper' => __DIR__ . '/../../..' . '/lib/private/Encryption/EncryptionWrapper.php',
         'OC\\Encryption\\Exceptions\\DecryptionFailedException' => __DIR__ . '/../../..' . '/lib/private/Encryption/Exceptions/DecryptionFailedException.php',
         'OC\\Encryption\\Exceptions\\EmptyEncryptionDataException' => __DIR__ . '/../../..' . '/lib/private/Encryption/Exceptions/EmptyEncryptionDataException.php',
@@ -1590,7 +1591,6 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OC\\Encryption\\Exceptions\\ModuleDoesNotExistsException' => __DIR__ . '/../../..' . '/lib/private/Encryption/Exceptions/ModuleDoesNotExistsException.php',
         'OC\\Encryption\\Exceptions\\UnknownCipherException' => __DIR__ . '/../../..' . '/lib/private/Encryption/Exceptions/UnknownCipherException.php',
         'OC\\Encryption\\File' => __DIR__ . '/../../..' . '/lib/private/Encryption/File.php',
-        'OC\\Encryption\\HookManager' => __DIR__ . '/../../..' . '/lib/private/Encryption/HookManager.php',
         'OC\\Encryption\\Keys\\Storage' => __DIR__ . '/../../..' . '/lib/private/Encryption/Keys/Storage.php',
         'OC\\Encryption\\Manager' => __DIR__ . '/../../..' . '/lib/private/Encryption/Manager.php',
         'OC\\Encryption\\Update' => __DIR__ . '/../../..' . '/lib/private/Encryption/Update.php',

lib/private/Encryption/EncryptionEventListener.php

@@ -0,0 +1,92 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+namespace OC\Encryption;
+
+use OC\Files\Filesystem;
+use OC\Files\SetupManager;
+use OC\Files\View;
+use OCA\Files_Trashbin\Events\NodeRestoredEvent;
+use OCP\Encryption\IFile;
+use OCP\EventDispatcher\Event;
+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;
+use OCP\Share\Events\ShareDeletedEvent;
+use Psr\Log\LoggerInterface;
+
+/** @template-implements IEventListener<NodeRenamedEvent|ShareCreatedEvent|ShareDeletedEvent|NodeRestoredEvent> */
+class EncryptionEventListener implements IEventListener {
+	private ?Update $updater = null;
+
+	public function __construct(
+		private IUserSession $userSession,
+		private SetupManager $setupManager,
+	) {
+	}
+
+	public static function register(IEventDispatcher $dispatcher): void {
+		$dispatcher->addServiceListener(NodeRenamedEvent::class, static::class);
+		$dispatcher->addServiceListener(ShareCreatedEvent::class, static::class);
+		$dispatcher->addServiceListener(ShareDeletedEvent::class, static::class);
+		$dispatcher->addServiceListener(NodeRestoredEvent::class, static::class);
+	}
+
+	public function handle(Event $event): void {
+		if ($event instanceof NodeRenamedEvent) {
+			$this->getUpdate()->postRename($event->getSource() instanceof Folder, $event->getSource()->getPath(), $event->getTarget()->getPath());
+		} elseif ($event instanceof ShareCreatedEvent) {
+			$this->getUpdate()->postShared($event->getShare()->getNodeType(), $event->getShare()->getNodeId());
+		} 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());
+		} elseif ($event instanceof NodeRestoredEvent) {
+			$this->getUpdate()->postRestore($event->getTarget() instanceof Folder, $event->getTarget()->getPath());
+		}
+	}
+
+	private function getUpdate(?IUser $owner = null): Update {
+		if (is_null($this->updater)) {
+			$user = $this->userSession->getUser();
+			if (!$user && ($owner !== null)) {
+				$user = $owner;
+			}
+			if (!$user) {
+				throw new \Exception('Inconsistent data, File unshared, but owner not found. Should not happen');
+			}
+
+			$uid = $user->getUID();
+
+			if (!$this->setupManager->isSetupComplete($user)) {
+				$this->setupManager->setupForUser($user);
+			}
+
+			$this->updater = new Update(
+				new Util(
+					new View(),
+					\OC::$server->getUserManager(),
+					\OC::$server->getGroupManager(),
+					\OC::$server->getConfig()),
+				Filesystem::getMountManager(),
+				\OC::$server->getEncryptionManager(),
+				\OC::$server->get(IFile::class),
+				\OC::$server->get(LoggerInterface::class),
+				$uid
+			);
+		}
+
+		return $this->updater;
+	}
+}

lib/private/Encryption/EncryptionWrapper.php

@@ -76,7 +76,6 @@ class EncryptionWrapper {
 				\OC::$server->getConfig()
 			);
 			$update = new Update(
-				new View(),
 				$util,
 				Filesystem::getMountManager(),
 				$this->manager,

lib/private/Encryption/HookManager.php

@@ -1,75 +0,0 @@
-<?php
-
-/**
- * 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 OC\Files\Filesystem;
-use OC\Files\SetupManager;
-use OC\Files\View;
-use OCP\Encryption\IFile;
-use Psr\Log\LoggerInterface;
-
-class HookManager {
-	private static ?Update $updater = null;
-
-	public static function postShared($params): void {
-		self::getUpdate()->postShared($params);
-	}
-	public static function postUnshared($params): void {
-		// In case the unsharing happens in a background job, we don't have
-		// a session and we load instead the user from the UserManager
-		$path = Filesystem::getPath($params['fileSource']);
-		$owner = Filesystem::getOwner($path);
-		self::getUpdate($owner)->postUnshared($params);
-	}
-
-	public static function postRename($params): void {
-		self::getUpdate()->postRename($params);
-	}
-
-	public static function postRestore($params): void {
-		self::getUpdate()->postRestore($params);
-	}
-
-	private static function getUpdate(?string $owner = null): Update {
-		if (is_null(self::$updater)) {
-			$user = \OC::$server->getUserSession()->getUser();
-			if (!$user && $owner) {
-				$user = \OC::$server->getUserManager()->get($owner);
-			}
-			if (!$user) {
-				throw new \Exception('Inconsistent data, File unshared, but owner not found. Should not happen');
-			}
-
-			$uid = '';
-			if ($user) {
-				$uid = $user->getUID();
-			}
-
-			$setupManager = \OC::$server->get(SetupManager::class);
-			if (!$setupManager->isSetupComplete($user)) {
-				$setupManager->setupForUser($user);
-			}
-
-			self::$updater = new Update(
-				new View(),
-				new Util(
-					new View(),
-					\OC::$server->getUserManager(),
-					\OC::$server->getGroupManager(),
-					\OC::$server->getConfig()),
-				Filesystem::getMountManager(),
-				\OC::$server->getEncryptionManager(),
-				\OC::$server->get(IFile::class),
-				\OC::$server->get(LoggerInterface::class),
-				$uid
-			);
-		}
-
-		return self::$updater;
-	}
-}

lib/private/Encryption/Update.php

@@ -18,9 +18,6 @@ use Psr\Log\LoggerInterface;
  * update encrypted files, e.g. because a file was shared
  */
 class Update {
-	/** @var View */
-	protected $view;
-
 	/** @var Util */
 	protected $util;
 
@@ -43,7 +40,6 @@ class Update {
 	 * @param string $uid
 	 */
 	public function __construct(
-		View $view,
 		Util $util,
 		Mount\Manager $mountManager,
 		Manager $encryptionManager,
@@ -51,7 +47,6 @@ class Update {
 		LoggerInterface $logger,
 		$uid,
 	) {
-		$this->view = $view;
 		$this->util = $util;
 		$this->mountManager = $mountManager;
 		$this->encryptionManager = $encryptionManager;
@@ -62,32 +57,28 @@ class Update {
 
 	/**
 	 * hook after file was shared
-	 *
-	 * @param array $params
 	 */
-	public function postShared($params) {
+	public function postShared(string $nodeType, int $nodeId): void {
 		if ($this->encryptionManager->isEnabled()) {
-			if ($params['itemType'] === 'file' || $params['itemType'] === 'folder') {
-				$path = Filesystem::getPath($params['fileSource']);
+			if ($nodeType === 'file' || $nodeType === 'folder') {
+				$path = Filesystem::getPath($nodeId);
 				[$owner, $ownerPath] = $this->getOwnerPath($path);
 				$absPath = '/' . $owner . '/files/' . $ownerPath;
-				$this->update($absPath);
+				$this->update($nodeType === 'folder', $absPath);
 			}
 		}
 	}
 
 	/**
 	 * hook after file was unshared
-	 *
-	 * @param array $params
 	 */
-	public function postUnshared($params) {
+	public function postUnshared(string $nodeType, int $nodeId): void {
 		if ($this->encryptionManager->isEnabled()) {
-			if ($params['itemType'] === 'file' || $params['itemType'] === 'folder') {
-				$path = Filesystem::getPath($params['fileSource']);
+			if ($nodeType === 'file' || $nodeType === 'folder') {
+				$path = Filesystem::getPath($nodeId);
 				[$owner, $ownerPath] = $this->getOwnerPath($path);
 				$absPath = '/' . $owner . '/files/' . $ownerPath;
-				$this->update($absPath);
+				$this->update($nodeType === 'folder', $absPath);
 			}
 		}
 	}
@@ -95,32 +86,26 @@ class Update {
 	/**
 	 * inform encryption module that a file was restored from the trash bin,
 	 * e.g. to update the encryption keys
-	 *
-	 * @param array $params
 	 */
-	public function postRestore($params) {
+	public function postRestore(bool $directory, string $filePath): void {
 		if ($this->encryptionManager->isEnabled()) {
-			$path = Filesystem::normalizePath('/' . $this->uid . '/files/' . $params['filePath']);
-			$this->update($path);
+			$path = Filesystem::normalizePath('/' . $this->uid . '/files/' . $filePath);
+			$this->update($directory, $path);
 		}
 	}
 
 	/**
 	 * inform encryption module that a file was renamed,
 	 * e.g. to update the encryption keys
-	 *
-	 * @param array $params
 	 */
-	public function postRename($params) {
-		$source = $params['oldpath'];
-		$target = $params['newpath'];
+	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($absPath);
+			$this->update($directory, $absPath);
 		}
 	}
 
@@ -149,7 +134,7 @@ class Update {
 	 * @param string $path relative to data/
 	 * @throws Exceptions\ModuleDoesNotExistsException
 	 */
-	public function update($path) {
+	public function update(bool $directory, string $path): void {
 		$encryptionModule = $this->encryptionManager->getEncryptionModule();
 
 		// if the encryption module doesn't encrypt the files on a per-user basis
@@ -159,7 +144,7 @@ class Update {
 		}
 
 		// if a folder was shared, get a list of all (sub-)folders
-		if ($this->view->is_dir($path)) {
+		if ($directory) {
 			$allFiles = $this->util->getAllFiles($path);
 		} else {
 			$allFiles = [$path];

tests/lib/Encryption/UpdateTest.php

@@ -13,36 +13,21 @@ use OC\Encryption\Util;
 use OC\Files\Mount\Manager;
 use OC\Files\View;
 use OCP\Encryption\IEncryptionModule;
+use PHPUnit\Framework\MockObject\MockObject;
 use Psr\Log\LoggerInterface;
 use Test\TestCase;
 
 class UpdateTest extends TestCase {
-	/** @var \OC\Encryption\Update */
-	private $update;
+	private Update $update;
 
-	/** @var string */
-	private $uid;
-
-	/** @var \OC\Files\View | \PHPUnit\Framework\MockObject\MockObject */
-	private $view;
-
-	/** @var Util | \PHPUnit\Framework\MockObject\MockObject */
-	private $util;
-
-	/** @var \OC\Files\Mount\Manager | \PHPUnit\Framework\MockObject\MockObject */
-	private $mountManager;
-
-	/** @var \OC\Encryption\Manager | \PHPUnit\Framework\MockObject\MockObject */
-	private $encryptionManager;
-
-	/** @var \OCP\Encryption\IEncryptionModule | \PHPUnit\Framework\MockObject\MockObject */
-	private $encryptionModule;
-
-	/** @var \OC\Encryption\File | \PHPUnit\Framework\MockObject\MockObject */
-	private $fileHelper;
-
-	/** @var \PHPUnit\Framework\MockObject\MockObject|LoggerInterface */
-	private $logger;
+	private string $uid;
+	private View&MockObject $view;
+	private Util&MockObject $util;
+	private Manager&MockObject $mountManager;
+	private \OC\Encryption\Manager&MockObject $encryptionManager;
+	private IEncryptionModule&MockObject $encryptionModule;
+	private File&MockObject $fileHelper;
+	private LoggerInterface&MockObject $logger;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -58,7 +43,6 @@ class UpdateTest extends TestCase {
 		$this->uid = 'testUser1';
 
 		$this->update = new Update(
-			$this->view,
 			$this->util,
 			$this->mountManager,
 			$this->encryptionManager,
@@ -80,10 +64,6 @@ class UpdateTest extends TestCase {
 			->method('getEncryptionModule')
 			->willReturn($this->encryptionModule);
 
-		$this->view->expects($this->once())
-			->method('is_dir')
-			->willReturn($isDir);
-
 		if ($isDir) {
 			$this->util->expects($this->once())
 				->method('getAllFiles')
@@ -98,7 +78,7 @@ class UpdateTest extends TestCase {
 			->method('update')
 			->willReturn(true);
 
-		$this->update->update($path);
+		$this->update->update($isDir, $path);
 	}
 
 	/**
@@ -143,7 +123,7 @@ class UpdateTest extends TestCase {
 			$updateMock->expects($this->once())->method('update');
 		}
 
-		$updateMock->postRename(['oldpath' => $source, 'newpath' => $target]);
+		$updateMock->postRename(false, $source, $target);
 	}
 
 	/**
@@ -181,7 +161,7 @@ class UpdateTest extends TestCase {
 			$updateMock->expects($this->never())->method('update');
 		}
 
-		$updateMock->postRestore(['filePath' => '/folder/test.txt']);
+		$updateMock->postRestore(false, '/folder/test.txt');
 	}
 
 	/**
@@ -200,13 +180,12 @@ class UpdateTest extends TestCase {
 	 * create mock of the update method
 	 *
 	 * @param array $methods methods which should be set
-	 * @return \OC\Encryption\Update | \PHPUnit\Framework\MockObject\MockObject
+	 * @return \OC\Encryption\Update | MockObject
 	 */
 	protected function getUpdateMock($methods) {
 		return  $this->getMockBuilder('\OC\Encryption\Update')
 			->setConstructorArgs(
 				[
-					$this->view,
 					$this->util,
 					$this->mountManager,
 					$this->encryptionManager,