Skip to content

Commit

Permalink
feat(encryption): Migrate from hooks to events
Browse files Browse the repository at this point in the history
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
  • Loading branch information
come-nc committed Nov 5, 2024
1 parent 093ed1e commit fc22188
Show file tree
Hide file tree
Showing 9 changed files with 133 additions and 153 deletions.
6 changes: 3 additions & 3 deletions apps/files_versions/lib/Listener/FileEventsListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ public function post_write_hook(Node $node): void {
/**
* 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 {
Expand Down Expand Up @@ -273,7 +273,7 @@ public function pre_remove_hook(Node $node): void {
/**
* 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 {
Expand All @@ -292,7 +292,7 @@ public function rename_hook(Node $source, Node $target): void {
/**
* 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 {
Expand Down
14 changes: 7 additions & 7 deletions lib/base.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,16 @@
* SPDX-FileCopyrightText: 2013-2016 ownCloud, Inc.
* SPDX-License-Identifier: AGPL-3.0-only
*/
use OC\Encryption\HookManager;
use OC\Share20\Hooks;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\Files\Events\BeforeFileSystemSetupEvent;
use OCP\Group\Events\UserRemovedEvent;
use OCP\ILogger;
use OCP\IRequest;
use OCP\IURLGenerator;
use OCP\IUserSession;
use OCP\Security\Bruteforce\IThrottler;
use OCP\Server;
use OCP\Share;
use OCP\User\Events\UserChangedEvent;
use Psr\Log\LoggerInterface;
use Symfony\Component\Routing\Exception\MethodNotAllowedException;
Expand Down Expand Up @@ -860,15 +859,16 @@ public static function registerCleanupHooks(\OC\SystemConfig $systemConfig): voi
}

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));
}
}

Expand Down
2 changes: 1 addition & 1 deletion lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -1463,6 +1463,7 @@
'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',
Expand All @@ -1473,7 +1474,6 @@
'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',
Expand Down
2 changes: 1 addition & 1 deletion lib/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -1496,6 +1496,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',
Expand All @@ -1506,7 +1507,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',
Expand Down
92 changes: 92 additions & 0 deletions lib/private/Encryption/EncryptionEventListener.php
Original file line number Diff line number Diff line change
@@ -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;
}
}
1 change: 0 additions & 1 deletion lib/private/Encryption/EncryptionWrapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ public function wrapStorage(string $mountPoint, IStorage $storage, IMountPoint $
\OC::$server->getConfig()
);
$update = new Update(
new View(),
$util,
Filesystem::getMountManager(),
$this->manager,
Expand Down
75 changes: 0 additions & 75 deletions lib/private/Encryption/HookManager.php

This file was deleted.

45 changes: 15 additions & 30 deletions lib/private/Encryption/Update.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@
* update encrypted files, e.g. because a file was shared
*/
class Update {
/** @var View */
protected $view;

/** @var Util */
protected $util;

Expand All @@ -43,15 +40,13 @@ class Update {
* @param string $uid
*/
public function __construct(
View $view,
Util $util,
Mount\Manager $mountManager,
Manager $encryptionManager,
File $file,
LoggerInterface $logger,
$uid,
) {
$this->view = $view;
$this->util = $util;
$this->mountManager = $mountManager;
$this->encryptionManager = $encryptionManager;
Expand All @@ -62,65 +57,55 @@ public function __construct(

/**
* 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);
}
}
}

/**
* 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);
}
}

Expand Down Expand Up @@ -149,7 +134,7 @@ protected function getOwnerPath($path) {
* @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
Expand All @@ -159,7 +144,7 @@ public function update($path) {
}

// 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];
Expand Down
Loading

0 comments on commit fc22188

Please # to comment.