Merge pull request #26820 from owncloud/setupfs-before-access-a-users…

…-keys-stable9

[stable9] Setupfs before access a users keys

lib/private/encryption/keys/storage.php

@@ -27,6 +27,8 @@
 use OC\Files\Filesystem;
 use OC\Files\View;
 use OCP\Encryption\Keys\IStorage;
+use OCP\IUserSession;
+use OC\User\NoUserException;
 
 class Storage implements IStorage {
 
@@ -53,17 +55,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();
+		}
 	}
 
 	/**
@@ -129,8 +139,21 @@ public function setSystemUserKey($keyId, $key, $encryptionModuleId) {
 	 * @inheritdoc
 	 */
 	public function deleteUserKey($uid, $keyId, $encryptionModuleId) {
-		$path = $this->constructUserKeyPath($encryptionModuleId, $keyId, $uid);
-		return !$this->view->file_exists($path) || $this->view->unlink($path);
+		try {
+			$path = $this->constructUserKeyPath($encryptionModuleId, $keyId, $uid);
+			return !$this->view->file_exists($path) || $this->view->unlink($path);
+		} catch (NoUserException $e) {
+			// this exception can come from initMountPoints() from setupUserMounts()
+			// for a deleted user.
+			//
+			// It means, that:
+			// - we are not running in alternative storage mode because we don't call
+			// initMountPoints() in that mode
+			// - the keys were in the user's home but since the user was deleted, the
+			// user's home is gone and so are the keys
+			//
+			// So there is nothing to do, just ignore.
+		}
 	}
 
 	/**
@@ -170,6 +193,7 @@ protected function constructUserKeyPath($encryptionModuleId, $keyId, $uid) {
 		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 +259,7 @@ private function getFileKeyDir($encryptionModuleId, $path) {
 		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 +323,7 @@ protected function getPathToKeys($path) {
 		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 +349,24 @@ protected function keySetPreparation($path) {
 		}
 	}
 
+	/**
+	 * 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 ($this->root_dir !== '') {
+			// this means that the keys are stored outside of the user's homes,
+			// so we don't need to mount anything
+			return;
+		}
+		if (!is_null($uid) && $uid !== '' && $uid !== $this->currentUser) {
+			\OC\Files\Filesystem::initMountPoints($uid);
+		}
+	}
+
 }

lib/private/server.php

@@ -142,7 +142,11 @@ public function __construct($webRoot, \OC\Config $config) {
 				$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/storage.php

@@ -24,17 +24,29 @@
 namespace Test\Encryption\Keys;
 
 use OC\Encryption\Keys\Storage;
+use OC\Encryption\Util;
+use OC\Files\View;
 use Test\TestCase;
+use Test\Traits\UserTrait;
 
+/**
+ * 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 */
@@ -47,6 +59,8 @@ public function setUp() {
 			->disableOriginalConstructor()
 			->getMock();
 
+		$this->util->method('getKeyStorageRoot')->willReturn('');
+
 		$this->view = $this->getMockBuilder('OC\Files\View')
 			->disableOriginalConstructor()
 			->getMock();
@@ -55,7 +69,19 @@ public function setUp() {
 			->disableOriginalConstructor()
 			->getMock();
 
-		$this->storage = new Storage($this->view, $this->util);
+		$user = $this->getMock('\OCP\IUser');
+		$user->method('getUID')->willReturn('user1');
+		$userSession = $this->getMock('\OCP\IUserSession');
+		$userSession->method('getUser')->willReturn($user);
+
+		$this->createUser('user1', '123456');
+		$this->createUser('user2', '123456');
+
+		$this->storage = new Storage($this->view, $this->util, $userSession);
+	}
+
+	public function tearDown() {
+		\OC\Files\Filesystem::tearDown();
 	}
 
 	public function testSetFileKey() {
@@ -237,6 +263,71 @@ public function testGetUserKey() {
 		);
 	}
 
+	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'));
+	}
+
+	public function testGetUserKeyWhenKeyStorageIsOutsideHome() {
+		$this->view->expects($this->once())
+			->method('file_get_contents')
+			->with($this->equalTo('/enckeys/user2/files_encryption/encModule/user2.publicKey'))
+			->willReturn('key');
+		$this->view->expects($this->once())
+			->method('file_exists')
+			->with($this->equalTo('/enckeys/user2/files_encryption/encModule/user2.publicKey'))
+			->willReturn(true);
+
+		$user = $this->getMock('\OCP\IUser');
+		$user->method('getUID')->willReturn('user1');
+		$userSession = $this->getMock('\OCP\IUserSession');
+		$userSession->method('getUser')->willReturn($user);
+		$util = $this->getMockBuilder('\OC\Encryption\Util')
+			->disableOriginalConstructor()
+			->getMock();
+		$util->method('getKeyStorageRoot')->willReturn('enckeys');
+		$storage = new Storage($this->view, $util, $userSession);
+
+		$this->assertFalse($this->isUserHomeMounted('user2'));
+
+		$this->assertSame('key',
+			$storage->getUserKey('user2', 'publicKey', 'encModule')
+		);
+
+		$this->assertFalse($this->isUserHomeMounted('user2'), 'mounting was not necessary');
+	}
+
+	/**
+	 * 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')