Merge pull request #15496 from owncloud/enc-check-if-key-exists-before-deleting

Check if the key exists, before trying to delete it

3 files changed, 29 insertions, 12 deletions

diff --git a/lib/private/encryption/keys/storage.php b/lib/private/encryption/keys/storage.php
index 40bd3056b1a..9d978193130 100644
--- a/lib/private/encryption/keys/storage.php
+++ b/lib/private/encryption/keys/storage.php
@@ -140,11 +140,11 @@ class Storage implements \OCP\Encryption\Keys\IStorage {
 	 * @param string $uid ID if the user for whom we want to delete the key
 	 * @param string $keyId id of the key
 	 *
-	 * @return boolean
+	 * @return boolean False when the key could not be deleted
 	 */
 	public function deleteUserKey($uid, $keyId) {
 		$path = $this->constructUserKeyPath($keyId, $uid);
-		return $this->view->unlink($path);
+		return !$this->view->file_exists($path) || $this->view->unlink($path);
 	}
 
 	/**
@@ -153,22 +153,23 @@ class Storage implements \OCP\Encryption\Keys\IStorage {
 	 * @param string $path path to file
 	 * @param string $keyId id of the key
 	 *
-	 * @return boolean
+	 * @return boolean False when the key could not be deleted
 	 */
 	public function deleteFileKey($path, $keyId) {
 		$keyDir = $this->getFileKeyDir($path);
-		return $this->view->unlink($keyDir . $keyId);
+		return !$this->view->file_exists($keyDir . $keyId) || $this->view->unlink($keyDir . $keyId);
 	}
 
 	/**
 	 * delete all file keys for a given file
 	 *
 	 * @param string $path to the file
-	 * @return boolean
+	 * @return boolean False when the key could not be deleted
 	 */
 	public function deleteAllFileKeys($path) {
 		$keyDir = $this->getFileKeyDir($path);
-		return $this->view->deleteAll(dirname($keyDir));
+		$path = dirname($keyDir);
+		return !$this->view->file_exists($path) || $this->view->deleteAll($path);
 	}
 
 	/**
@@ -177,11 +178,11 @@ class Storage implements \OCP\Encryption\Keys\IStorage {
 	 *
 	 * @param string $keyId id of the key
 	 *
-	 * @return boolean
+	 * @return boolean False when the key could not be deleted
 	 */
 	public function deleteSystemUserKey($keyId) {
 		$path = $this->constructUserKeyPath($keyId);
-		return $this->view->unlink($path);
+		return !$this->view->file_exists($path) || $this->view->unlink($path);
 	}
 
 
diff --git a/lib/public/encryption/keys/istorage.php b/lib/public/encryption/keys/istorage.php
index 898ab81c373..c6933e7afab 100644
--- a/lib/public/encryption/keys/istorage.php
+++ b/lib/public/encryption/keys/istorage.php
@@ -89,7 +89,7 @@ interface IStorage {
 	 * @param string $uid ID if the user for whom we want to delete the key
 	 * @param string $keyId id of the key
 	 *
-	 * @return boolean
+	 * @return boolean False when the key could not be deleted
 	 */
 	public function deleteUserKey($uid, $keyId);
 
@@ -99,7 +99,7 @@ interface IStorage {
 	 * @param string $path path to file
 	 * @param string $keyId id of the key
 	 *
-	 * @return boolean
+	 * @return boolean False when the key could not be deleted
 	 */
 	public function deleteFileKey($path, $keyId);
 
@@ -107,7 +107,7 @@ interface IStorage {
 	 * delete all file keys for a given file
 	 *
 	 * @param string $path to the file
-	 * @return boolean
+	 * @return boolean False when the keys could not be deleted
 	 */
 	public function deleteAllFileKeys($path);
 
@@ -117,7 +117,7 @@ interface IStorage {
 	 *
 	 * @param string $keyId id of the key
 	 *
-	 * @return boolean
+	 * @return boolean False when the key could not be deleted
 	 */
 	public function deleteSystemUserKey($keyId);
 
diff --git a/tests/lib/encryption/keys/storage.php b/tests/lib/encryption/keys/storage.php
index 8ab46987f8c..bcf1c0f7624 100644
--- a/tests/lib/encryption/keys/storage.php
+++ b/tests/lib/encryption/keys/storage.php
@@ -198,6 +198,10 @@ class StorageTest extends TestCase {
 
 	public function testDeleteUserKey() {
 		$this->view->expects($this->once())
+			->method('file_exists')
+			->with($this->equalTo('/user1/files_encryption/encModule/user1.publicKey'))
+			->willReturn(true);
+		$this->view->expects($this->once())
 			->method('unlink')
 			->with($this->equalTo('/user1/files_encryption/encModule/user1.publicKey'))
 			->willReturn(true);
@@ -209,6 +213,10 @@ class StorageTest extends TestCase {
 
 	public function testDeleteSystemUserKey() {
 		$this->view->expects($this->once())
+			->method('file_exists')
+			->with($this->equalTo('/files_encryption/encModule/shareKey_56884'))
+			->willReturn(true);
+		$this->view->expects($this->once())
 			->method('unlink')
 			->with($this->equalTo('/files_encryption/encModule/shareKey_56884'))
 			->willReturn(true);
@@ -229,6 +237,10 @@ class StorageTest extends TestCase {
 			->method('isSystemWideMountPoint')
 			->willReturn(true);
 		$this->view->expects($this->once())
+			->method('file_exists')
+			->with($this->equalTo('/files_encryption/keys/files/foo.txt/encModule/fileKey'))
+			->willReturn(true);
+		$this->view->expects($this->once())
 			->method('unlink')
 			->with($this->equalTo('/files_encryption/keys/files/foo.txt/encModule/fileKey'))
 			->willReturn(true);
@@ -249,6 +261,10 @@ class StorageTest extends TestCase {
 			->method('isSystemWideMountPoint')
 			->willReturn(false);
 		$this->view->expects($this->once())
+			->method('file_exists')
+			->with($this->equalTo('/user1/files_encryption/keys/files/foo.txt/encModule/fileKey'))
+			->willReturn(true);
+		$this->view->expects($this->once())
 			->method('unlink')
 			->with($this->equalTo('/user1/files_encryption/keys/files/foo.txt/encModule/fileKey'))
 			->willReturn(true);