Merge pull request #11835 from owncloud/enc-fixmovingsharekeystofolder

Fix moving share keys as non-owner to subdir

apps/files_encryption/hooks/hooks.php

@@ -409,34 +409,18 @@ class Hooks {
 	 * @param array $params with the old path and the new path
 	 */
 	public static function preRename($params) {
-		$user = \OCP\User::getUser();
-		$view = new \OC\Files\View('/');
-		$util = new Util($view, $user);
-		list($ownerOld, $pathOld) = $util->getUidAndFilename($params['oldpath']);
-
-		// we only need to rename the keys if the rename happens on the same mountpoint
-		// otherwise we perform a stream copy, so we get a new set of keys
-		$mp1 = $view->getMountPoint('/' . $user . '/files/' . $params['oldpath']);
-		$mp2 = $view->getMountPoint('/' . $user . '/files/' . $params['newpath']);
-
-		$type = $view->is_dir('/' . $user . '/files/' . $params['oldpath']) ? 'folder' : 'file';
-
-		if ($mp1 === $mp2) {
-			self::$renamedFiles[$params['oldpath']] = array(
-				'uid' => $ownerOld,
-				'path' => $pathOld,
-				'type' => $type,
-				'operation' => 'rename',
-				);
-
-		}
+		self::preRenameOrCopy($params, 'rename');
 	}
 
 	/**
-	 * mark file as renamed so that we know the original source after the file was renamed
+	 * mark file as copied so that we know the original source after the file was copied
 	 * @param array $params with the old path and the new path
 	 */
 	public static function preCopy($params) {
+		self::preRenameOrCopy($params, 'copy');
+	}
+
+	private static function preRenameOrCopy($params, $operation) {
 		$user = \OCP\User::getUser();
 		$view = new \OC\Files\View('/');
 		$util = new Util($view, $user);
@@ -450,11 +434,27 @@ class Hooks {
 		$type = $view->is_dir('/' . $user . '/files/' . $params['oldpath']) ? 'folder' : 'file';
 
 		if ($mp1 === $mp2) {
+			if ($util->isSystemWideMountPoint($pathOld)) {
+				$oldShareKeyPath = 'files_encryption/share-keys/' . $pathOld;
+			} else {
+				$oldShareKeyPath = $ownerOld . '/' . 'files_encryption/share-keys/' . $pathOld;
+			}
+			// gather share keys here because in postRename() the file will be moved already
+			$oldShareKeys = Helper::findShareKeys($pathOld, $oldShareKeyPath, $view);
+			if (count($oldShareKeys) === 0) {
+				\OC_Log::write(
+					'Encryption library', 'No share keys found for "' . $pathOld . '"',
+					\OC_Log::WARN
+				);
+			}
 			self::$renamedFiles[$params['oldpath']] = array(
 				'uid' => $ownerOld,
 				'path' => $pathOld,
 				'type' => $type,
-				'operation' => 'copy');
+				'operation' => $operation,
+				'sharekeys' => $oldShareKeys
+				);
+
 		}
 	}
 
@@ -476,6 +476,7 @@ class Hooks {
 		$view = new \OC\Files\View('/');
 		$userId = \OCP\User::getUser();
 		$util = new Util($view, $userId);
+		$oldShareKeys = null;
 
 		if (isset(self::$renamedFiles[$params['oldpath']]['uid']) &&
 				isset(self::$renamedFiles[$params['oldpath']]['path'])) {
@@ -483,6 +484,7 @@ class Hooks {
 			$pathOld = self::$renamedFiles[$params['oldpath']]['path'];
 			$type =  self::$renamedFiles[$params['oldpath']]['type'];
 			$operation = self::$renamedFiles[$params['oldpath']]['operation'];
+			$oldShareKeys = self::$renamedFiles[$params['oldpath']]['sharekeys'];
 			unset(self::$renamedFiles[$params['oldpath']]);
 		} else {
 			\OCP\Util::writeLog('Encryption library', "can't get path and owner from the file before it was renamed", \OCP\Util::DEBUG);
@@ -522,15 +524,7 @@ class Hooks {
 			$oldKeyfilePath .= '.key';
 			$newKeyfilePath .= '.key';
 
-			// handle share-keys
-			$matches = Helper::findShareKeys($pathOld, $oldShareKeyPath, $view);
-			if (count($matches) === 0) {
-				\OC_Log::write(
-					'Encryption library', 'No share keys found for "' . $pathOld . '"',
-					\OC_Log::WARN
-				);
-			}
-			foreach ($matches as $src) {
+			foreach ($oldShareKeys as $src) {
 				$dst = \OC\Files\Filesystem::normalizePath(str_replace($pathOld, $pathNew, $src));
 				$view->$operation($src, $dst);
 			}

apps/files_encryption/tests/share.php

@@ -1074,8 +1074,19 @@ class Test_Encryption_Share extends \PHPUnit_Framework_TestCase {
 		\OC\Files\Filesystem::unlink('/newfolder');
 	}
 
-	function testMoveFileToFolder() {
+	function usersProvider() {
+		return array(
+			// test as owner
+			array(\Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER1),
+			// test as share receiver
+			array(\Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2),
+		);
+	}
 
+	/**
+	 * @dataProvider usersProvider
+	 */
+	function testMoveFileToFolder($userId) {
 		$view = new \OC\Files\View('/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER1);
 
 		$filename = '/tmp-' . uniqid();
@@ -1108,8 +1119,10 @@ class Test_Encryption_Share extends \PHPUnit_Framework_TestCase {
 		$this->assertTrue($view->file_exists('files_encryption/share-keys' . $folder . '/' . $filename . '.' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER1 . '.shareKey'));
 		$this->assertTrue($view->file_exists('files_encryption/share-keys' . $folder . '/' . $filename . '.' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '.shareKey'));
 
-		// move the file into the subfolder
+		// move the file into the subfolder as the test user
+		\Test_Encryption_Util::loginHelper($userId);
 		\OC\Files\Filesystem::rename($folder . $filename, $subFolder . $filename);
+		\Test_Encryption_Util::loginHelper(\Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER1);
 
 		// Get file decrypted contents
 		$newDecrypt = \OC\Files\Filesystem::file_get_contents($subFolder . $filename);

apps/files_sharing/lib/sharedstorage.php

@@ -300,7 +300,7 @@ class Shared extends \OC\Files\Storage\Common implements ISharedStorage {
 			$pathinfo = pathinfo($relPath1);
 			// for part files we need to ask for the owner and path from the parent directory because
 			// the file cache doesn't return any results for part files
-			if ($pathinfo['extension'] === 'part') {
+			if (isset($pathinfo['extension']) && $pathinfo['extension'] === 'part') {
 				list($user1, $path1) = \OCA\Files_Sharing\Helper::getUidAndFilename($pathinfo['dirname']);
 				$path1 = $path1 . '/' . $pathinfo['basename'];
 			} else {