Merge pull request #26061 from nextcloud/bugfix/noid/encryption-updat…

…e-failure

Log and continue when failing to update encryption keys during for individual files

lib/private/Encryption/EncryptionWrapper.php

@@ -31,6 +31,7 @@
 use OCP\Files\Mount\IMountPoint;
 use OCP\Files\Storage;
 use OCP\ILogger;
+use Psr\Log\LoggerInterface;
 
 /**
  * Class EncryptionWrapper
@@ -100,6 +101,7 @@ public function wrapStorage($mountPoint, Storage $storage, IMountPoint $mount) {
 				Filesystem::getMountManager(),
 				$this->manager,
 				$fileHelper,
+				\OC::$server->get(LoggerInterface::class),
 				$uid
 			);
 			return new Encryption(

lib/private/Encryption/HookManager.php

@@ -25,6 +25,7 @@
 
 use OC\Files\Filesystem;
 use OC\Files\View;
+use Psr\Log\LoggerInterface;
 
 class HookManager {
 	/**
@@ -67,6 +68,7 @@ private static function getUpdate() {
 				Filesystem::getMountManager(),
 				\OC::$server->getEncryptionManager(),
 				\OC::$server->getEncryptionFilesHelper(),
+				\OC::$server->get(LoggerInterface::class),
 				$uid
 			);
 		}

lib/private/Encryption/Update.php

@@ -26,40 +26,40 @@
 
 namespace OC\Encryption;
 
+use InvalidArgumentException;
 use OC\Files\Filesystem;
 use OC\Files\Mount;
 use OC\Files\View;
+use OCP\Encryption\Exceptions\GenericEncryptionException;
+use Psr\Log\LoggerInterface;
 
 /**
  * update encrypted files, e.g. because a file was shared
  */
 class Update {
 
-	/** @var \OC\Files\View */
+	/** @var View */
 	protected $view;
 
-	/** @var \OC\Encryption\Util */
+	/** @var Util */
 	protected $util;
 
 	/** @var \OC\Files\Mount\Manager */
 	protected $mountManager;
 
-	/** @var \OC\Encryption\Manager */
+	/** @var Manager */
 	protected $encryptionManager;
 
 	/** @var string */
 	protected $uid;
 
-	/** @var \OC\Encryption\File */
+	/** @var File */
 	protected $file;
 
+	/** @var LoggerInterface */
+	protected $logger;
+
 	/**
-	 *
-	 * @param \OC\Files\View $view
-	 * @param \OC\Encryption\Util $util
-	 * @param \OC\Files\Mount\Manager $mountManager
-	 * @param \OC\Encryption\Manager $encryptionManager
-	 * @param \OC\Encryption\File $file
 	 * @param string $uid
 	 */
 	public function __construct(
@@ -68,13 +68,15 @@ public function __construct(
 			Mount\Manager $mountManager,
 			Manager $encryptionManager,
 			File $file,
+			LoggerInterface $logger,
 			$uid
 		) {
 		$this->view = $view;
 		$this->util = $util;
 		$this->mountManager = $mountManager;
 		$this->encryptionManager = $encryptionManager;
 		$this->file = $file;
+		$this->logger = $logger;
 		$this->uid = $uid;
 	}
 
@@ -155,7 +157,7 @@ protected function getOwnerPath($path) {
 		$view = new View('/' . $owner . '/files');
 		$path = $view->getPath($info->getId());
 		if ($path === null) {
-			throw new \InvalidArgumentException('No file found for ' . $info->getId());
+			throw new InvalidArgumentException('No file found for ' . $info->getId());
 		}
 
 		return [$owner, $path];
@@ -187,7 +189,12 @@ public function update($path) {
 
 		foreach ($allFiles as $file) {
 			$usersSharing = $this->file->getAccessList($file);
-			$encryptionModule->update($file, $this->uid, $usersSharing);
+			try {
+				$encryptionModule->update($file, $this->uid, $usersSharing);
+			} catch (GenericEncryptionException $e) {
+				// If the update of an individual file fails e.g. due to a corrupt key we should continue the operation and just log the failure
+				$this->logger->error('Failed to update encryption module for ' . $this->uid . ' ' . $file, [ 'exception' => $e ]);
+			}
 		}
 	}
 }

tests/lib/Encryption/UpdateTest.php

@@ -22,9 +22,13 @@
 namespace Test\Encryption;
 
 use OC\Encryption\Update;
+use OC\Encryption\Util;
 use OC\Files\Mount\Manager;
 use OC\Files\View;
+use Psr\Log\LoggerInterface;
 use Test\TestCase;
+use OC\Encryption\File;
+use OCP\Encryption\IEncryptionModule;
 
 class UpdateTest extends TestCase {
 
@@ -37,7 +41,7 @@ class UpdateTest extends TestCase {
 	/** @var \OC\Files\View | \PHPUnit\Framework\MockObject\MockObject */
 	private $view;
 
-	/** @var \OC\Encryption\Util | \PHPUnit\Framework\MockObject\MockObject */
+	/** @var Util | \PHPUnit\Framework\MockObject\MockObject */
 	private $util;
 
 	/** @var \OC\Files\Mount\Manager | \PHPUnit\Framework\MockObject\MockObject */
@@ -52,21 +56,19 @@ class UpdateTest extends TestCase {
 	/** @var \OC\Encryption\File | \PHPUnit\Framework\MockObject\MockObject */
 	private $fileHelper;
 
+	/** @var \PHPUnit\Framework\MockObject\MockObject|LoggerInterface */
+	private $logger;
+
 	protected function setUp(): void {
 		parent::setUp();
 
-		$this->view = $this->getMockBuilder(View::class)
-			->disableOriginalConstructor()->getMock();
-		$this->util = $this->getMockBuilder('\OC\Encryption\Util')
-			->disableOriginalConstructor()->getMock();
-		$this->mountManager = $this->getMockBuilder(Manager::class)
-			->disableOriginalConstructor()->getMock();
-		$this->encryptionManager = $this->getMockBuilder('\OC\Encryption\Manager')
-			->disableOriginalConstructor()->getMock();
-		$this->fileHelper = $this->getMockBuilder('\OC\Encryption\File')
-			->disableOriginalConstructor()->getMock();
-		$this->encryptionModule = $this->getMockBuilder('\OCP\Encryption\IEncryptionModule')
-			->disableOriginalConstructor()->getMock();
+		$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';
 
@@ -76,6 +78,7 @@ protected function setUp(): void {
 			$this->mountManager,
 			$this->encryptionManager,
 			$this->fileHelper,
+			$this->logger,
 			$this->uid);
 	}
 
@@ -223,6 +226,7 @@ protected function getUpdateMock($methods) {
 					$this->mountManager,
 					$this->encryptionManager,
 					$this->fileHelper,
+					$this->logger,
 					$this->uid
 				]
 			)->setMethods($methods)->getMock();