From 0d5c7a11e287f44ad556d29b952c57c9ec11dfaa Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Mon, 11 May 2015 10:35:42 +0200 Subject: [PATCH] use hooks to update encryption keys instead of the storage wrapper if a file gets renamed/restored, as long as we are in the storage wrapper the file cache isn't up-to-date --- lib/base.php | 2 + lib/private/encryption/hookmanager.php | 8 ++ lib/private/encryption/update.php | 34 +++++- .../files/storage/wrapper/encryption.php | 8 +- tests/lib/encryption/updatetest.php | 115 +++++++++++++++++- .../lib/files/storage/wrapper/encryption.php | 11 +- 6 files changed, 156 insertions(+), 22 deletions(-) diff --git a/lib/base.php b/lib/base.php index a9d582dcc3..d1ff9fe799 100644 --- a/lib/base.php +++ b/lib/base.php @@ -727,6 +727,8 @@ class OC { if ($enabled) { \OCP\Util::connectHook('OCP\Share', 'post_shared', 'OC\Encryption\HookManager', 'postShared'); \OCP\Util::connectHook('OCP\Share', 'post_unshare', 'OC\Encryption\HookManager', 'postUnshared'); + \OCP\Util::connectHook('OC_Filesystem', 'post_rename', 'OC\Encryption\HookManager', 'postRename'); + \OCP\Util::connectHook('\OCA\Files_Trashbin\Trashbin', 'post_restore', 'OC\Encryption\HookManager', 'postRestore'); } } diff --git a/lib/private/encryption/hookmanager.php b/lib/private/encryption/hookmanager.php index c62583b4b4..31ecb2fbcf 100644 --- a/lib/private/encryption/hookmanager.php +++ b/lib/private/encryption/hookmanager.php @@ -37,6 +37,14 @@ class HookManager { self::getUpdate()->postUnshared($params); } + public static function postRename($params) { + self::getUpdate()->postRename($params); + } + + public static function postRestore($params) { + self::getUpdate()->postRestore($params); + } + /** * @return Update */ diff --git a/lib/private/encryption/update.php b/lib/private/encryption/update.php index ddcee3bae9..02579fd913 100644 --- a/lib/private/encryption/update.php +++ b/lib/private/encryption/update.php @@ -107,6 +107,38 @@ 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) { + if ($this->encryptionManager->isEnabled()) { + $path = Filesystem::normalizePath('/' . $this->uid . '/files/' . $params['filePath']); + $this->update($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']; + if( + $this->encryptionManager->isEnabled() && + dirname($source) !== dirname($target) + ) { + list($owner, $ownerPath) = $this->getOwnerPath($target); + $absPath = '/' . $owner . '/files/' . $ownerPath; + $this->update($absPath); + } + } + /** * get owner and path relative to data//files * @@ -114,7 +146,7 @@ class Update { * @return array ['owner' => $owner, 'path' => $path] * @throw \InvalidArgumentException */ - private function getOwnerPath($path) { + protected function getOwnerPath($path) { $info = Filesystem::getFileInfo($path); $owner = Filesystem::getOwner($path); $view = new View('/' . $owner . '/files'); diff --git a/lib/private/files/storage/wrapper/encryption.php b/lib/private/files/storage/wrapper/encryption.php index c0c4c6979c..c42e6d439f 100644 --- a/lib/private/files/storage/wrapper/encryption.php +++ b/lib/private/files/storage/wrapper/encryption.php @@ -231,13 +231,7 @@ class Encryption extends Wrapper { if (isset($this->unencryptedSize[$source])) { $this->unencryptedSize[$target] = $this->unencryptedSize[$source]; } - $keysRenamed = $this->keyStorage->renameKeys($source, $target); - if ($keysRenamed && - dirname($source) !== dirname($target) && - $this->util->isFile($target) - ) { - $this->update->update($target); - } + $this->keyStorage->renameKeys($source, $target); } } diff --git a/tests/lib/encryption/updatetest.php b/tests/lib/encryption/updatetest.php index 790d071aa6..b222bc42d7 100644 --- a/tests/lib/encryption/updatetest.php +++ b/tests/lib/encryption/updatetest.php @@ -68,10 +68,6 @@ class UpdateTest extends TestCase { $this->encryptionModule = $this->getMockBuilder('\OCP\Encryption\IEncryptionModule') ->disableOriginalConstructor()->getMock(); - $this->encryptionManager->expects($this->once()) - ->method('getEncryptionModule') - ->willReturn($this->encryptionModule); - $this->uid = 'testUser1'; $this->update = new Update( @@ -93,6 +89,10 @@ class UpdateTest extends TestCase { */ public function testUpdate($path, $isDir, $allFiles, $numberOfFiles) { + $this->encryptionManager->expects($this->once()) + ->method('getEncryptionModule') + ->willReturn($this->encryptionModule); + $this->view->expects($this->once()) ->method('is_dir') ->willReturn($isDir); @@ -126,4 +126,111 @@ class UpdateTest extends TestCase { ); } + /** + * @dataProvider dataTestPostRename + * + * @param string $source + * @param string $target + * @param boolean $encryptionEnabled + */ + public function testPostRename($source, $target, $encryptionEnabled) { + + $updateMock = $this->getUpdateMock(['update', 'getOwnerPath']); + + $this->encryptionManager->expects($this->once()) + ->method('isEnabled') + ->willReturn($encryptionEnabled); + + if (dirname($source) === dirname($target) || $encryptionEnabled === false) { + $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->postRename(['oldpath' => $source, 'newpath' => $target]); + } + + /** + * test data for testPostRename() + * + * @return array + */ + public function dataTestPostRename() { + return array( + array('/test.txt', '/testNew.txt', true), + array('/test.txt', '/testNew.txt', false), + array('/folder/test.txt', '/testNew.txt', true), + array('/folder/test.txt', '/testNew.txt', false), + array('/folder/test.txt', '/testNew.txt', true), + array('/test.txt', '/folder/testNew.txt', false), + ); + } + + + /** + * @dataProvider dataTestPostRestore + * + * @param boolean $encryptionEnabled + */ + public function testPostRestore($encryptionEnabled) { + + $updateMock = $this->getUpdateMock(['update']); + + $this->encryptionManager->expects($this->once()) + ->method('isEnabled') + ->willReturn($encryptionEnabled); + + if ($encryptionEnabled) { + $updateMock->expects($this->once())->method('update'); + + } else { + $updateMock->expects($this->never())->method('update'); + } + + $updateMock->postRestore(['filePath' => '/folder/test.txt']); + } + + /** + * test data for testPostRestore() + * + * @return array + */ + public function dataTestPostRestore() { + return array( + array(true), + array(false), + ); + } + + /** + * create mock of the update method + * + * @param array$methods methods which should be set + * @return \OC\Encryption\Update | \PHPUnit_Framework_MockObject_MockObject + */ + protected function getUpdateMock($methods) { + return $this->getMockBuilder('\OC\Encryption\Update') + ->setConstructorArgs( + [ + $this->view, + $this->util, + $this->mountManager, + $this->encryptionManager, + $this->fileHelper, + $this->uid + ] + )->setMethods($methods)->getMock(); + } + } diff --git a/tests/lib/files/storage/wrapper/encryption.php b/tests/lib/files/storage/wrapper/encryption.php index d4492e0092..97810c9c5d 100644 --- a/tests/lib/files/storage/wrapper/encryption.php +++ b/tests/lib/files/storage/wrapper/encryption.php @@ -157,13 +157,11 @@ class Encryption extends \Test\Files\Storage\Storage { * @param string $target * @param $encryptionEnabled * @param boolean $renameKeysReturn - * @param boolean $shouldUpdate */ public function testRename($source, $target, $encryptionEnabled, - $renameKeysReturn, - $shouldUpdate) { + $renameKeysReturn) { if ($encryptionEnabled) { $this->keyStore ->expects($this->once()) @@ -177,13 +175,6 @@ class Encryption extends \Test\Files\Storage\Storage { ->method('isFile')->willReturn(true); $this->encryptionManager->expects($this->once()) ->method('isEnabled')->willReturn($encryptionEnabled); - if ($shouldUpdate) { - $this->update->expects($this->once()) - ->method('update'); - } else { - $this->update->expects($this->never()) - ->method('update'); - } $this->instance->mkdir($source); $this->instance->mkdir(dirname($target));