Fix file upload to ext storage when recovery key is enabled

Fixes an issue when uploading files to external storage when recovery
keys are enabled

The Util class only works with real users, so instantiating it with the
virtual recovery key user or public key user can cause issues.

2 files changed, 58 insertions, 9 deletions

diff --git a/apps/files_encryption/lib/util.php b/apps/files_encryption/lib/util.php
index d12b003b227..d214d13de69 100644
--- a/apps/files_encryption/lib/util.php
+++ b/apps/files_encryption/lib/util.php
@@ -869,6 +869,25 @@ class Util {
 	}
 
 	/**
+	 * Returns whether the given user is ready for encryption.
+	 * Also returns true if the given user is the public user
+	 * or the recovery key user.
+	 *
+	 * @param string $user user to check
+	 *
+	 * @return boolean true if the user is ready, false otherwise
+	 */
+	private function isUserReady($user) {
+		if ($user === $this->publicShareKeyId
+			|| $user === $this->recoveryKeyId
+		) {
+			return true;
+		}
+		$util = new Util($this->view, $user);
+		return $util->ready();
+	}
+
+	/**
 	 * Filter an array of UIDs to return only ones ready for sharing
 	 * @param array $unfilteredUsers users to be checked for sharing readiness
 	 * @return array as multi-dimensional array. keys: ready, unready
@@ -880,16 +899,9 @@ class Util {
 
 		// Loop through users and create array of UIDs that need new keyfiles
 		foreach ($unfilteredUsers as $user) {
-
-			$util = new Util($this->view, $user);
-
 			// Check that the user is encryption capable, or is the
-			// public system user 'ownCloud' (for public shares)
-			if (
-				$user === $this->publicShareKeyId
-				or $user === $this->recoveryKeyId
-				or $util->ready()
-			) {
+			// public system user (for public shares)
+			if ($this->isUserReady($user)) {
 
 				// Construct array of ready UIDs for Keymanager{}
 				$readyIds[] = $user;
diff --git a/apps/files_encryption/tests/util.php b/apps/files_encryption/tests/util.php
index bbf6efae5b9..b8057202a07 100755
--- a/apps/files_encryption/tests/util.php
+++ b/apps/files_encryption/tests/util.php
@@ -550,6 +550,43 @@ class Test_Encryption_Util extends \OCA\Files_Encryption\Tests\TestCase {
 	}
 
 	/**
+	 * Tests that filterShareReadyUsers() returns the correct list of
+	 * users that are ready or not ready for encryption
+	 */
+	public function testFilterShareReadyUsers() {
+		$appConfig = \OC::$server->getAppConfig();
+
+		$publicShareKeyId = $appConfig->getValue('files_encryption', 'publicShareKeyId');
+		$recoveryKeyId = $appConfig->getValue('files_encryption', 'recoveryKeyId');
+
+		$usersToTest = array(
+			'readyUser',
+			'notReadyUser',
+			'nonExistingUser',
+			$publicShareKeyId,
+			$recoveryKeyId,
+		);
+		\Test_Encryption_Util::loginHelper('readyUser', true);
+		\Test_Encryption_Util::loginHelper('notReadyUser', true);
+		// delete encryption dir to make it not ready
+		$this->view->unlink('notReadyUser/files_encryption/');
+
+		// login as user1
+		\Test_Encryption_Util::loginHelper(\Test_Encryption_Util::TEST_ENCRYPTION_UTIL_USER1);
+
+		$result = $this->util->filterShareReadyUsers($usersToTest);
+		$this->assertEquals(
+			array('readyUser', $publicShareKeyId, $recoveryKeyId),
+			$result['ready']
+		);
+		$this->assertEquals(
+			array('notReadyUser', 'nonExistingUser'),
+			$result['unready']
+		);
+		\OC_User::deleteUser('readyUser');
+	}
+
+	/**
 	 * @param string $user
 	 * @param bool $create
 	 * @param bool $password