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

Check if the key exists, before trying to delete it

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);
 	}
 
 

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);
 

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'))