upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-04-20 22:00:39 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 39

fix(encryption): Improve Update class and event listenening

Browse source 
to avoid back&forth between path and Node object

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
This commit is contained in:
Côme Chilliet 2025-02-11 16:06:33 +01:00 committed by Côme Chilliet
parent 2d8f6b366a
commit a79b5dea7c
6 changed files with 113 additions and 185 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
lib/private/Encryption/EncryptionEventListener.php
View file

@ -9,7 +9,6 @@ declare(strict_types=1);
namespace OC\Encryption;
use OC\Files\Filesystem;
use OC\Files\SetupManager;
use OC\Files\View;
use OCA\Files_Trashbin\Events\NodeRestoredEvent;
@ -18,7 +17,6 @@ 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;
@ -32,6 +30,7 @@ class EncryptionEventListener implements IEventListener {
public function __construct(
private IUserSession $userSession,
private SetupManager $setupManager,
private Manager $encryptionManager,
) {
}
@ -43,17 +42,20 @@ class EncryptionEventListener implements IEventListener {
}
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());
}
}
@ -79,7 +81,6 @@ class EncryptionEventListener implements IEventListener {
\OC::$server->getUserManager(),
\OC::$server->getGroupManager(),
\OC::$server->getConfig()),
Filesystem::getMountManager(),
\OC::$server->getEncryptionManager(),
\OC::$server->get(IFile::class),
\OC::$server->get(LoggerInterface::class),

9
lib/private/Encryption/EncryptionWrapper.php
View file

@ -75,14 +75,6 @@ class EncryptionWrapper {
\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 @@ class EncryptionWrapper {
$fileHelper,
$uid,
$keyStorage,
$update,
$mountManager,
$this->arrayCache
);

117
lib/private/Encryption/Update.php
View file

@ -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);
$view = new View('/' . $owner . '/files');
$path = $view->getPath($info->getId());
if ($path === null) {
throw new InvalidArgumentException('No file found for ' . $info->getId());
protected function getOwnerPath(OCPFile|Folder $node): string {
$owner = $node->getOwner()?->getUID();
if ($owner === null) {
throw new InvalidArgumentException('No owner found for ' . $node->getId());
}
return [$owner, $path];
$view = new View('/' . $owner . '/files');
try {
$path = $view->getPath($node->getId());
} catch (NotFoundException $e) {
throw new InvalidArgumentException('No file found for ' . $node->getId(), previous:$e);
}
return '/' . $owner . '/files/' . $path;
}
/**
@ -134,7 +88,7 @@ class Update {
* @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 @@ class Update {
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 {

2
lib/private/Files/Storage/Wrapper/Encryption.php
View file

@ -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;
@ -50,7 +49,6 @@ class Encryption extends Wrapper {
private IFile $fileHelper,
private ?string $uid,
private IStorage $keyStorage,
private Update $update,
private Manager $mountManager,
private ArrayCache $arrayCache,
) {

140
tests/lib/Encryption/UpdateTest.php
View file

@ -1,4 +1,7 @@
<?php
declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2017-2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-FileCopyrightText: 2016 ownCloud, Inc.
@ -10,20 +13,19 @@ namespace Test\Encryption;
use OC\Encryption\File;
use OC\Encryption\Update;
use OC\Encryption\Util;
use OC\Files\Mount\Manager;
use OC\Files\View;
use OCP\Encryption\IEncryptionModule;
use OCP\Files\File as OCPFile;
use OCP\Files\Folder;
use OCP\IUser;
use PHPUnit\Framework\MockObject\MockObject;
use Psr\Log\LoggerInterface;
use Test\TestCase;
class UpdateTest extends TestCase {
private Update $update;
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;
@ -34,21 +36,44 @@ class UpdateTest extends TestCase {
$this->view = $this->createMock(View::class);
$this->util = $this->createMock(Util::class);
$this->mountManager = $this->createMock(Manager::class);
$this->encryptionManager = $this->createMock(\OC\Encryption\Manager::class);
$this->fileHelper = $this->createMock(File::class);
$this->encryptionModule = $this->createMock(IEncryptionModule::class);
$this->logger = $this->createMock(LoggerInterface::class);
$this->uid = 'testUser1';
}
$this->update = new Update(
$this->util,
$this->mountManager,
$this->encryptionManager,
$this->fileHelper,
$this->logger,
$this->uid);
private function getUserMock(string $uid): IUser&MockObject {
$user = $this->createMock(IUser::class);
$user->expects(self::any())
->method('getUID')
->willReturn($uid);
return $user;
}
private function getFileMock(string $path, string $owner): OCPFile&MockObject {
$node = $this->createMock(OCPFile::class);
$node->expects(self::atLeastOnce())
->method('getPath')
->willReturn($path);
$node->expects(self::any())
->method('getOwner')
->willReturn($this->getUserMock($owner));
return $node;
}
private function getFolderMock(string $path, string $owner): Folder&MockObject {
$node = $this->createMock(Folder::class);
$node->expects(self::atLeastOnce())
->method('getPath')
->willReturn($path);
$node->expects(self::any())
->method('getOwner')
->willReturn($this->getUserMock($owner));
return $node;
}
/**
@ -60,6 +85,10 @@ class UpdateTest extends TestCase {
* @param integer $numberOfFiles
*/
public function testUpdate($path, $isDir, $allFiles, $numberOfFiles): void {
$updateMock = $this->getUpdateMock(['getOwnerPath']);
$updateMock->expects($this->once())->method('getOwnerPath')
->willReturnCallback(fn (OCPFile|Folder $node) => '/user/' . $node->getPath());
$this->encryptionManager->expects($this->once())
->method('getEncryptionModule')
->willReturn($this->encryptionModule);
@ -68,6 +97,9 @@ class UpdateTest extends TestCase {
$this->util->expects($this->once())
->method('getAllFiles')
->willReturn($allFiles);
$node = $this->getFolderMock($path, 'user');
} else {
$node = $this->getFileMock($path, 'user');
}
$this->fileHelper->expects($this->exactly($numberOfFiles))
@ -78,7 +110,7 @@ class UpdateTest extends TestCase {
->method('update')
->willReturn(true);
$this->update->update($isDir, $path);
$updateMock->update($node);
}
/**
@ -98,32 +130,26 @@ class UpdateTest extends TestCase {
*
* @param string $source
* @param string $target
* @param boolean $encryptionEnabled
*/
public function testPostRename($source, $target, $encryptionEnabled): void {
$updateMock = $this->getUpdateMock(['update', 'getOwnerPath']);
public function testPostRename($source, $target): void {
$updateMock = $this->getUpdateMock(['update','getOwnerPath']);
$this->encryptionManager->expects($this->once())
->method('isEnabled')
->willReturn($encryptionEnabled);
$sourceNode = $this->getFileMock($source, 'user');
$targetNode = $this->getFileMock($target, 'user');
if (dirname($source) === dirname($target) || $encryptionEnabled === false) {
if (dirname($source) === dirname($target)) {
$updateMock->expects($this->never())->method('getOwnerPath');
$updateMock->expects($this->never())->method('update');
} else {
$updateMock->expects($this->once())
->method('getOwnerPath')
->willReturnCallback(function ($path) use ($target) {
$this->assertSame(
$target,
$path,
'update needs to be executed for the target destination');
return ['owner', $path];
});
$updateMock->expects($this->once())->method('update');
$updateMock->expects($this->once())->method('update')
->willReturnCallback(fn (OCPFile|Folder $node) => $this->assertSame(
$target,
$node->getPath(),
'update needs to be executed for the target destination'
));
}
$updateMock->postRename(false, $source, $target);
$updateMock->postRename($sourceNode, $targetNode);
}
/**
@ -133,61 +159,35 @@ class UpdateTest extends TestCase {
*/
public function dataTestPostRename() {
return [
['/test.txt', '/testNew.txt', true],
['/test.txt', '/testNew.txt', false],
['/folder/test.txt', '/testNew.txt', true],
['/folder/test.txt', '/testNew.txt', false],
['/folder/test.txt', '/testNew.txt', true],
['/test.txt', '/folder/testNew.txt', false],
['/test.txt', '/testNew.txt'],
['/folder/test.txt', '/testNew.txt'],
['/test.txt', '/folder/testNew.txt'],
];
}
/**
* @dataProvider dataTestPostRestore
*
* @param boolean $encryptionEnabled
*/
public function testPostRestore($encryptionEnabled): void {
public function testPostRestore(): void {
$updateMock = $this->getUpdateMock(['update']);
$this->encryptionManager->expects($this->once())
->method('isEnabled')
->willReturn($encryptionEnabled);
$updateMock->expects($this->once())->method('update')
->willReturnCallback(fn (OCPFile|Folder $node) => $this->assertSame(
'/folder/test.txt',
$node->getPath(),
'update needs to be executed for the target destination'
));
if ($encryptionEnabled) {
$updateMock->expects($this->once())->method('update');
} else {
$updateMock->expects($this->never())->method('update');
}
$updateMock->postRestore(false, '/folder/test.txt');
}
/**
* test data for testPostRestore()
*
* @return array
*/
public function dataTestPostRestore() {
return [
[true],
[false],
];
$updateMock->postRestore($this->getFileMock('/folder/test.txt', 'user'));
}
/**
* create mock of the update method
*
* @param array $methods methods which should be set
* @return \OC\Encryption\Update | MockObject
*/
protected function getUpdateMock($methods) {
return $this->getMockBuilder('\OC\Encryption\Update')
protected function getUpdateMock(array $methods): Update&MockObject {
return $this->getMockBuilder(Update::class)
->setConstructorArgs(
[
$this->util,
$this->mountManager,
$this->encryptionManager,
$this->fileHelper,
$this->logger,

15
tests/lib/Files/Storage/Wrapper/EncryptionTest.php
View file

@ -11,7 +11,6 @@ use Exception;
use OC;
use OC\Encryption\Exceptions\ModuleDoesNotExistsException;
use OC\Encryption\File;
use OC\Encryption\Update;
use OC\Encryption\Util;
use OC\Files\Cache\Cache;
use OC\Files\Cache\CacheEntry;
@ -46,7 +45,6 @@ class EncryptionTest extends Storage {
private Util&MockObject $util;
private \OC\Encryption\Manager&MockObject $encryptionManager;
private IEncryptionModule&MockObject $encryptionModule;
private Update&MockObject $update;
private Cache&MockObject $cache;
private LoggerInterface&MockObject $logger;
private File&MockObject $file;
@ -111,9 +109,6 @@ class EncryptionTest extends Storage {
$this->keyStore = $this->getMockBuilder('\OC\Encryption\Keys\Storage')
->disableOriginalConstructor()->getMock();
$this->update = $this->getMockBuilder('\OC\Encryption\Update')
->disableOriginalConstructor()->getMock();
$this->mount = $this->getMockBuilder('\OC\Files\Mount\MountPoint')
->disableOriginalConstructor()
->setMethods(['getOption'])
@ -155,7 +150,6 @@ class EncryptionTest extends Storage {
$this->file,
null,
$this->keyStore,
$this->update,
$this->mountManager,
$this->arrayCache
]
@ -237,7 +231,6 @@ class EncryptionTest extends Storage {
$this->file,
null,
$this->keyStore,
$this->update,
$this->mountManager,
$this->arrayCache,
]
@ -316,7 +309,6 @@ class EncryptionTest extends Storage {
$this->file,
null,
$this->keyStore,
$this->update,
$this->mountManager,
$this->arrayCache,
]
@ -361,7 +353,6 @@ class EncryptionTest extends Storage {
$this->file,
null,
$this->keyStore,
$this->update,
$this->mountManager,
$this->arrayCache,
]
@ -491,7 +482,6 @@ class EncryptionTest extends Storage {
$this->file,
null,
$this->keyStore,
$this->update,
$this->mountManager,
$this->arrayCache,
);
@ -598,7 +588,6 @@ class EncryptionTest extends Storage {
$this->file,
null,
$this->keyStore,
$this->update,
$this->mountManager,
$this->arrayCache,
]
@ -692,7 +681,6 @@ class EncryptionTest extends Storage {
$this->file,
null,
$this->keyStore,
$this->update,
$this->mountManager,
$this->arrayCache,
]
@ -867,7 +855,6 @@ class EncryptionTest extends Storage {
$this->file,
null,
$this->keyStore,
$this->update,
$this->mountManager,
$this->arrayCache
]
@ -968,7 +955,6 @@ class EncryptionTest extends Storage {
$util = $this->createMock(Util::class);
$fileHelper = $this->createMock(IFile::class);
$keyStorage = $this->createMock(IStorage::class);
$update = $this->createMock(Update::class);
$mountManager = $this->createMock(\OC\Files\Mount\Manager::class);
$mount = $this->createMock(IMountPoint::class);
$arrayCache = $this->createMock(ArrayCache::class);
@ -986,7 +972,6 @@ class EncryptionTest extends Storage {
$fileHelper,
null,
$keyStorage,
$update,
$mountManager,
$arrayCache
]