Merge pull request #16228 from owncloud/enc_fix_restore

use hooks to update encryption keys

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

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
 	 */

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/<owner>/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');

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

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

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