From 45575d0135368169af71379a39c8914e7a1cf524 Mon Sep 17 00:00:00 2001
From: Joas Schilling <nickvergessen@owncloud.com>
Date: Thu, 9 Apr 2015 10:28:02 +0200
Subject: [PATCH] Check if the key exists, before trying to delete it

---
 lib/private/encryption/keys/storage.php | 17 +++++++++--------
 lib/public/encryption/keys/istorage.php |  8 ++++----
 tests/lib/encryption/keys/storage.php   | 16 ++++++++++++++++
 3 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/lib/private/encryption/keys/storage.php b/lib/private/encryption/keys/storage.php
index 40bd3056b1..9d97819313 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 898ab81c37..c6933e7afa 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 8ab46987f8..bcf1c0f762 100644
--- a/tests/lib/encryption/keys/storage.php
+++ b/tests/lib/encryption/keys/storage.php
@@ -197,6 +197,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'))
@@ -208,6 +212,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'))
@@ -228,6 +236,10 @@ class StorageTest extends TestCase {
 		$this->util->expects($this->any())
 			->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'))
@@ -248,6 +260,10 @@ class StorageTest extends TestCase {
 		$this->util->expects($this->any())
 			->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'))