Merge pull request #22796 from owncloud/fix-encryption-on-version-restore

Keep "encryptedVersion" when calling `\OC\Files\View::copy`

apps/files_versions/lib/storage.php

@@ -191,12 +191,7 @@ class Storage {
 			$mtime = $users_view->filemtime('files/' . $filename);
 			$users_view->copy('files/' . $filename, 'files_versions/' . $filename . '.v' . $mtime);
 			// call getFileInfo to enforce a file cache entry for the new version
-			$newFileInfo = $users_view->getFileInfo('files_versions/' . $filename . '.v' . $mtime);
+			$users_view->getFileInfo('files_versions/' . $filename . '.v' . $mtime);
-
-			// Keep the "encrypted" value of the original file
-			$oldVersion = $files_view->getFileInfo($filename)->getEncryptedVersion();
-			$cache = $newFileInfo->getStorage()->getCache();
-			$cache->update($newFileInfo->getId(), ['encrypted' => $oldVersion, 'encryptedVersion' => $oldVersion]);
 		}
 	}
 
@@ -331,15 +326,22 @@ class Storage {
 
 			//first create a new version
 			$version = 'files_versions'.$filename.'.v'.$users_view->filemtime('files'.$filename);
-			if ( !$users_view->file_exists($version)) {
+			if (!$users_view->file_exists($version)) {
-
 				$users_view->copy('files'.$filename, 'files_versions'.$filename.'.v'.$users_view->filemtime('files'.$filename));
-
 				$versionCreated = true;
 			}
 
+			$fileToRestore =  'files_versions' . $filename . '.v' . $revision;
+
+			// Restore encrypted version of the old file for the newly restored file
+			// This has to happen manually here since the file is manually copied below
+			$oldVersion = $users_view->getFileInfo($fileToRestore)->getEncryptedVersion();
+			$newFileInfo = $files_view->getFileInfo($filename);
+			$cache = $newFileInfo->getStorage()->getCache();
+			$cache->update($newFileInfo->getId(), ['encrypted' => $oldVersion, 'encryptedVersion' => $oldVersion]);
+
 			// rollback
-			if (self::copyFileContents($users_view, 'files_versions' . $filename . '.v' . $revision, 'files' . $filename)) {
+			if (self::copyFileContents($users_view, $fileToRestore, 'files' . $filename)) {
 				$files_view->touch($file, $revision);
 				Storage::scheduleExpire($uid, $file);
 				\OC_Hook::emit('\OCP\Versions', 'rollback', array(

lib/private/files/storage/wrapper/encryption.php

@@ -620,6 +620,32 @@ class Encryption extends Wrapper {
 		return $this->copyBetweenStorage($sourceStorage, $sourceInternalPath, $targetInternalPath, $preserveMtime, false);
 	}
 
+	/**
+	 * Update the encrypted cache version in the database
+	 *
+	 * @param Storage $sourceStorage
+	 * @param string $sourceInternalPath
+	 * @param string $targetInternalPath
+	 * @param bool $isRename
+	 */
+	private function updateEncryptedVersion(Storage $sourceStorage, $sourceInternalPath, $targetInternalPath, $isRename) {
+		$isEncrypted = $this->encryptionManager->isEnabled() && $this->mount->getOption('encrypt', true) ? 1 : 0;
+		$cacheInformation = [
+			'encrypted' => (bool)$isEncrypted,
+		];
+		if($isEncrypted === 1) {
+			$cacheInformation['encryptedVersion'] = $sourceStorage->getCache()->get($sourceInternalPath)['encryptedVersion'];
+		}
+
+		// in case of a rename we need to manipulate the source cache because
+		// this information will be kept for the new target
+		if ($isRename) {
+			$sourceStorage->getCache()->put($sourceInternalPath, $cacheInformation);
+		} else {
+			$this->getCache()->put($targetInternalPath, $cacheInformation);
+		}
+	}
+
 	/**
 	 * copy file between two storages
 	 *
@@ -647,6 +673,7 @@ class Encryption extends Wrapper {
 						$info['size']
 					);
 				}
+				$this->updateEncryptedVersion($sourceStorage, $sourceInternalPath, $targetInternalPath, $isRename);
 			}
 			return $result;
 		}
@@ -689,15 +716,7 @@ class Encryption extends Wrapper {
 				if ($preserveMtime) {
 					$this->touch($targetInternalPath, $sourceStorage->filemtime($sourceInternalPath));
 				}
-				$isEncrypted = $this->encryptionManager->isEnabled() && $this->mount->getOption('encrypt', true) ? 1 : 0;
+				$this->updateEncryptedVersion($sourceStorage, $sourceInternalPath, $targetInternalPath, $isRename);
-
-				// in case of a rename we need to manipulate the source cache because
-				// this information will be kept for the new target
-				if ($isRename) {
-					$sourceStorage->getCache()->put($sourceInternalPath, ['encrypted' => $isEncrypted]);
-				} else {
-					$this->getCache()->put($targetInternalPath, ['encrypted' => $isEncrypted]);
-				}
 			} else {
 				// delete partially written target file
 				$this->unlink($targetInternalPath);

tests/lib/files/storage/wrapper/encryption.php

@@ -693,11 +693,19 @@ class Encryption extends Storage {
 				$temp = \OC::$server->getTempManager();
 				return fopen($temp->getTemporaryFile(), $mode);
 			});
-
+		if($expectedEncrypted) {
+			$cache = $this->getMock('\OCP\Files\Cache\ICache');
+			$cache->expects($this->once())
+				->method('get')
+				->with($sourceInternalPath)
+				->willReturn(['encryptedVersion' => 12345]);
+			$storage2->expects($this->once())
+				->method('getCache')
+				->willReturn($cache);
+		}
 		$this->encryptionManager->expects($this->any())
 			->method('isEnabled')
 			->willReturn($encryptionEnabled);
-
 		// FIXME can not overwrite the return after definition
 //		$this->mount->expects($this->at(0))
 //			->method('getOption')
@@ -706,9 +714,16 @@ class Encryption extends Storage {
 		global $mockedMountPointEncryptionEnabled;
 		$mockedMountPointEncryptionEnabled = $mountPointEncryptionEnabled;
 
+		$expectedCachePut = [
+			'encrypted' => $expectedEncrypted,
+		];
+		if($expectedEncrypted === true) {
+			$expectedCachePut['encryptedVersion'] = 12345;
+		}
+
 		$this->cache->expects($this->once())
 			->method('put')
-			->with($sourceInternalPath, ['encrypted' => $expectedEncrypted]);
+			->with($sourceInternalPath, $expectedCachePut);
 
 		$this->invokePrivate($this->instance, 'copyBetweenStorage', [$storage2, $sourceInternalPath, $targetInternalPath, $preserveMtime, $isRename]);
 
@@ -765,10 +780,10 @@ class Encryption extends Storage {
 			->with($sourceStorage, $sourceInternalPath, $targetInternalPath)
 			->willReturn($copyResult);
 
+		$instance->expects($this->any())->method('getCache')
+			->willReturn($cache);
+
 		if ($copyResult) {
-			$instance->expects($this->once())->method('getCache')
-				->with('', $sourceStorage)
-				->willReturn($cache);
 			$cache->expects($this->once())->method('get')
 				->with($sourceInternalPath)
 				->willReturn(['encrypted' => $encrypted, 'size' => 42]);