Merge pull request #3841 from nextcloud/encyryption-trash-bin

Delete files on encryption error

4 files changed, 34 insertions, 14 deletions

diff --git a/apps/encryption/lib/Crypto/Crypt.php b/apps/encryption/lib/Crypto/Crypt.php
index 7d6636d882e..4303cb9e940 100644
--- a/apps/encryption/lib/Crypto/Crypt.php
+++ b/apps/encryption/lib/Crypto/Crypt.php
@@ -29,7 +29,6 @@ namespace OCA\Encryption\Crypto;
 
 use OC\Encryption\Exceptions\DecryptionFailedException;
 use OC\Encryption\Exceptions\EncryptionFailedException;
-use OC\HintException;
 use OCA\Encryption\Exceptions\MultiKeyDecryptException;
 use OCA\Encryption\Exceptions\MultiKeyEncryptException;
 use OCP\Encryption\Exceptions\GenericEncryptionException;
@@ -476,12 +475,12 @@ class Crypt {
 	 * @param string $data
 	 * @param string $passPhrase
 	 * @param string $expectedSignature
-	 * @throws HintException
+	 * @throws GenericEncryptionException
 	 */
 	private function checkSignature($data, $passPhrase, $expectedSignature) {
 		$signature = $this->createSignature($data, $passPhrase);
 		if (!hash_equals($expectedSignature, $signature)) {
-			throw new HintException('Bad Signature', $this->l->t('Bad Signature'));
+			throw new GenericEncryptionException('Bad Signature', $this->l->t('Bad Signature'));
 		}
 	}
 
@@ -552,7 +551,7 @@ class Crypt {
 	 * @param string $catFile
 	 * @param string $cipher
 	 * @return bool
-	 * @throws HintException
+	 * @throws GenericEncryptionException
 	 */
 	private function hasSignature($catFile, $cipher) {
 		$meta = substr($catFile, -93);
@@ -560,7 +559,7 @@ class Crypt {
 
 		// enforce signature for the new 'CTR' ciphers
 		if ($signaturePosition === false && strpos(strtolower($cipher), 'ctr') !== false) {
-			throw new HintException('Missing Signature', $this->l->t('Missing Signature'));
+			throw new GenericEncryptionException('Missing Signature', $this->l->t('Missing Signature'));
 		}
 
 		return ($signaturePosition !== false);
diff --git a/apps/encryption/tests/Crypto/CryptTest.php b/apps/encryption/tests/Crypto/CryptTest.php
index b808acaf199..3c226ed94ab 100644
--- a/apps/encryption/tests/Crypto/CryptTest.php
+++ b/apps/encryption/tests/Crypto/CryptTest.php
@@ -247,7 +247,7 @@ class CryptTest extends TestCase {
 
 	/**
 	 * @dataProvider dataTestHasSignatureFail
-	 * @expectedException \OC\HintException
+	 * @expectedException \OCP\Encryption\Exceptions\GenericEncryptionException
 	 */
 	public function testHasSignatureFail($cipher) {
 		$data = 'encryptedContent00iv001234567890123456xx';
diff --git a/apps/files_trashbin/lib/Storage.php b/apps/files_trashbin/lib/Storage.php
index b8f154ea051..e3fe648281c 100644
--- a/apps/files_trashbin/lib/Storage.php
+++ b/apps/files_trashbin/lib/Storage.php
@@ -28,6 +28,8 @@ namespace OCA\Files_Trashbin;
 use OC\Files\Filesystem;
 use OC\Files\Storage\Wrapper\Wrapper;
 use OC\Files\View;
+use OCP\Encryption\Exceptions\GenericEncryptionException;
+use OCP\ILogger;
 use OCP\IUserManager;
 
 class Storage extends Wrapper {
@@ -55,15 +57,21 @@ class Storage extends Wrapper {
 	/** @var  IUserManager */
 	private $userManager;
 
+	/** @var ILogger */
+	private $logger;
+
 	/**
 	 * Storage constructor.
 	 *
 	 * @param array $parameters
 	 * @param IUserManager|null $userManager
 	 */
-	public function __construct($parameters, IUserManager $userManager = null) {
+	public function __construct($parameters,
+								IUserManager $userManager = null,
+								ILogger $logger = null) {
 		$this->mountPoint = $parameters['mountPoint'];
 		$this->userManager = $userManager;
+		$this->logger = $logger;
 		parent::__construct($parameters);
 	}
 
@@ -147,11 +155,20 @@ class Storage extends Wrapper {
 	 * @return bool true if the operation succeeded, false otherwise
 	 */
 	public function unlink($path) {
-		if (isset(self::$moveOutOfSharedFolder[$this->mountPoint . $path])) {
-			$result = $this->doDelete($path, 'unlink', true);
-			unset(self::$moveOutOfSharedFolder[$this->mountPoint . $path]);
-		} else {
-			$result = $this->doDelete($path, 'unlink');
+		try {
+			if (isset(self::$moveOutOfSharedFolder[$this->mountPoint . $path])) {
+				$result = $this->doDelete($path, 'unlink', true);
+				unset(self::$moveOutOfSharedFolder[$this->mountPoint . $path]);
+			} else {
+				$result = $this->doDelete($path, 'unlink');
+			}
+		} catch (GenericEncryptionException $e) {
+			// in case of a encryption exception we delete the file right away
+			$this->logger->info(
+				"Can't move file" .  $path .
+				"to the trash bin, therefore it was deleted right away");
+
+			$result = $this->storage->unlink($path);
 		}
 
 		return $result;
@@ -251,7 +268,8 @@ class Storage extends Wrapper {
 		\OC\Files\Filesystem::addStorageWrapper('oc_trashbin', function ($mountPoint, $storage) {
 			return new \OCA\Files_Trashbin\Storage(
 				array('storage' => $storage, 'mountPoint' => $mountPoint),
-				\OC::$server->getUserManager()
+				\OC::$server->getUserManager(),
+				\OC::$server->getLogger()
 			);
 		}, 1);
 	}
diff --git a/apps/files_trashbin/tests/StorageTest.php b/apps/files_trashbin/tests/StorageTest.php
index 2aebe6414f7..52d5f056148 100644
--- a/apps/files_trashbin/tests/StorageTest.php
+++ b/apps/files_trashbin/tests/StorageTest.php
@@ -31,6 +31,7 @@ namespace OCA\Files_Trashbin\Tests;
 
 use OC\Files\Storage\Temporary;
 use OC\Files\Filesystem;
+use OCP\ILogger;
 
 /**
  * Class Storage
@@ -528,9 +529,11 @@ class StorageTest extends \Test\TestCase {
 			->disableOriginalConstructor()->getMock();
 		$userManager->expects($this->any())
 			->method('userExists')->willReturn($userExists);
+		$logger = $this->getMockBuilder(ILogger::class)->getMock();
 		$storage = new \OCA\Files_Trashbin\Storage(
 			['mountPoint' => $mountPoint, 'storage' => $tmpStorage],
-			$userManager
+			$userManager,
+			$logger
 		);
 
 		$this->assertSame($expected,