Setupfs before access a users keys (#26917)

* The file system of a user has to be properly setup before accessing the keys

* Fix unit test execution

* Init mount points for user in more places in Keys\Storage

* Fix encryption key storage tests to properly create required users

* Added test that check if user home is mounted after resolving key

lib/private/Encryption/Keys/Storage.php

@@ -27,6 +27,7 @@ use OC\Encryption\Util;
 use OC\Files\Filesystem;
 use OC\Files\View;
 use OCP\Encryption\Keys\IStorage;
+use OCP\IUserSession;
 
 class Storage implements IStorage {
 
@@ -53,17 +54,25 @@ class Storage implements IStorage {
 	/** @var array */
 	private $keyCache = [];
 
+	/** @var string */
+	private $currentUser = null;
+
 	/**
-	 * @param View $view
-	 * @param Util $util
+	 * @param View $view view
+	 * @param Util $util encryption util class
+	 * @param IUserSession $session user session
 	 */
-	public function __construct(View $view, Util $util) {
+	public function __construct(View $view, Util $util, IUserSession $session) {
 		$this->view = $view;
 		$this->util = $util;
 
 		$this->encryption_base_dir = '/files_encryption';
 		$this->keys_base_dir = $this->encryption_base_dir .'/keys';
 		$this->root_dir = $this->util->getKeyStorageRoot();
+
+		if (!is_null($session) && !is_null($session->getUser())) {
+			$this->currentUser = $session->getUser()->getUID();
+		}
 	}
 
 	/**
@@ -170,6 +179,7 @@ class Storage implements IStorage {
 		if ($uid === null) {
 			$path = $this->root_dir . '/' . $this->encryption_base_dir . '/' . $encryptionModuleId . '/' . $keyId;
 		} else {
+			$this->setupUserMounts($uid);
 			$path = $this->root_dir . '/' . $uid . $this->encryption_base_dir . '/'
 				. $encryptionModuleId . '/' . $uid . '.' . $keyId;
 		}
@@ -235,6 +245,7 @@ class Storage implements IStorage {
 		if ($this->util->isSystemWideMountPoint($filename, $owner)) {
 			$keyPath = $this->root_dir . '/' . $this->keys_base_dir . $filename . '/';
 		} else {
+			$this->setupUserMounts($owner);
 			$keyPath = $this->root_dir . '/' . $owner . $this->keys_base_dir . $filename . '/';
 		}
 
@@ -298,6 +309,7 @@ class Storage implements IStorage {
 		if ($systemWideMountPoint) {
 			$systemPath = $this->root_dir . '/' . $this->keys_base_dir . $relativePath . '/';
 		} else {
+			$this->setupUserMounts($owner);
 			$systemPath = $this->root_dir . '/' . $owner . $this->keys_base_dir . $relativePath . '/';
 		}
 
@@ -323,4 +335,19 @@ class Storage implements IStorage {
 		}
 	}
 
+	/**
+	 * Setup the mounts of the given user if different than
+	 * the current user.
+	 *
+	 * This is needed because in many cases the keys are stored
+	 * within the user's home storage.
+	 *
+	 * @param string $uid user id
+	 */
+	protected function setupUserMounts($uid) {
+		if (!is_null($uid) && $uid !== '' && $uid !== $this->currentUser) {
+			\OC\Files\Filesystem::initMountPoints($uid);
+		}
+	}
+
 }

lib/private/Server.php

@@ -156,7 +156,11 @@ class Server extends ServerContainer implements IServerContainer {
 				$c->getConfig()
 			);
 
-			return new Encryption\Keys\Storage($view, $util);
+			return new Encryption\Keys\Storage(
+				$view,
+				$util,
+				$c->getUserSession()
+			);
 		});
 		$this->registerService('TagMapper', function (Server $c) {
 			return new TagMapper($c->getDatabaseConnection());

tests/lib/Encryption/Keys/StorageTest.php

@@ -24,17 +24,31 @@
 namespace Test\Encryption\Keys;
 
 use OC\Encryption\Keys\Storage;
+use OC\Encryption\Util;
+use OC\Files\View;
 use Test\TestCase;
+use Test\Traits\UserTrait;
+use OCP\IUser;
+use OCP\IUserSession;
 
+/**
+ * Class StorageTest
+ *
+ * @group DB
+ *
+ * @package Test\Encryption\Keys
+ */
 class StorageTest extends TestCase {
 
+	use UserTrait;
+
 	/** @var Storage */
 	protected $storage;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject */
+	/** @var \PHPUnit_Framework_MockObject_MockObject | Util */
 	protected $util;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject */
+	/** @var \PHPUnit_Framework_MockObject_MockObject | View */
 	protected $view;
 
 	/** @var \PHPUnit_Framework_MockObject_MockObject */
@@ -55,7 +69,14 @@ class StorageTest extends TestCase {
 			->disableOriginalConstructor()
 			->getMock();
 
-		$this->storage = new Storage($this->view, $this->util);
+		$user = $this->createMock(IUser::class);
+		$user->method('getUID')->willReturn('user1');
+		$userSession = $this->createMock(IUserSession::class);
+		$userSession->method('getUser')->willReturn($user);
+
+		$this->createUser('user1', '123456');
+		$this->createUser('user2', '123456');
+		$this->storage = new Storage($this->view, $this->util, $userSession);
 	}
 
 	public function testSetFileKey() {
@@ -237,6 +258,42 @@ class StorageTest extends TestCase {
 		);
 	}
 
+	public function testGetUserKeyShared() {
+		$this->view->expects($this->once())
+			->method('file_get_contents')
+			->with($this->equalTo('/user2/files_encryption/encModule/user2.publicKey'))
+			->willReturn('key');
+		$this->view->expects($this->once())
+			->method('file_exists')
+			->with($this->equalTo('/user2/files_encryption/encModule/user2.publicKey'))
+			->willReturn(true);
+
+		$this->assertFalse($this->isUserHomeMounted('user2'));
+
+		$this->assertSame('key',
+			$this->storage->getUserKey('user2', 'publicKey', 'encModule')
+		);
+
+		$this->assertTrue($this->isUserHomeMounted('user2'));
+	}
+
+	/**
+	 * Returns whether the home of the given user was mounted
+	 *
+	 * @param string $userId
+	 * @return boolean true if mounted, false otherwise
+	 */
+	private function isUserHomeMounted($userId) {
+		// verify that user2's FS got mounted when retrieving the key
+		$mountManager = \OC::$server->getMountManager();
+		$mounts = $mountManager->getAll();
+		$mounts = array_filter($mounts, function($mount) use ($userId) {
+			return ($mount->getMountPoint() === "/$userId/");
+		});
+
+		return !empty($mounts);
+	}
+
 	public function testDeleteUserKey() {
 		$this->view->expects($this->once())
 			->method('file_exists')