Added password obfuscation for external storage config

Added obfuscation for all "password" options from external storages.
Added more ext storage unit tests config.
Added unit tests for reading/writing the configuration.
Added IV for ext storage password encryption
Moved the mounting code for external storage from
OC\Filesystem::initMountPoint to files_external using the post_initMountPoints hook
Fixed ext storage unit test for groups

Squashed backport of 2c561c9c5072ce82e06bd5ab2c4ee81bc5d09d59 from
master.

4 files changed, 618 insertions, 24 deletions

diff --git a/apps/files_external/appinfo/app.php b/apps/files_external/appinfo/app.php
index f78f3abf0fa..c2ce1580a71 100644
--- a/apps/files_external/appinfo/app.php
+++ b/apps/files_external/appinfo/app.php
@@ -24,5 +24,6 @@ if (OCP\Config::getAppValue('files_external', 'allow_user_mounting', 'yes') == '
 }
 
 // connecting hooks
-OCP\Util::connectHook( 'OC_User', 'post_login', 'OC\Files\Storage\iRODS', 'login' );
+OCP\Util::connectHook('OC_Filesystem', 'post_initMountPoints', '\OC_Mount_Config', 'initMountPointsHook');
+OCP\Util::connectHook('OC_User', 'post_login', 'OC\Files\Storage\iRODS', 'login');
 
diff --git a/apps/files_external/lib/config.php b/apps/files_external/lib/config.php
index 9896429464a..a20a2a02f1d 100755
--- a/apps/files_external/lib/config.php
+++ b/apps/files_external/lib/config.php
@@ -4,6 +4,7 @@
 *
 * @author Michael Gapczynski
 * @copyright 2012 Michael Gapczynski mtgap@owncloud.com
+* @copyright 2014 Vincent Petry <pvince81@owncloud.com>
 *
 * This library is free software; you can redistribute it and/or
 * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
@@ -19,15 +20,24 @@
 * License along with this library.  If not, see <http://www.gnu.org/licenses/>.
 */
 
+set_include_path(
+	get_include_path() . PATH_SEPARATOR .
+	\OC_App::getAppPath('files_external') . '/3rdparty/phpseclib/phpseclib'
+);
+
 /**
-* Class to configure the config/mount.php and data/$user/mount.php files
-*/
+ * Class to configure mount.json globally and for users
+ */
 class OC_Mount_Config {
+	// TODO: make this class non-static and give it a proper namespace
 
 	const MOUNT_TYPE_GLOBAL = 'global';
 	const MOUNT_TYPE_GROUP = 'group';
 	const MOUNT_TYPE_USER = 'user';
 
+	// whether to skip backend test (for unit tests, as this static class is not mockable)
+	public static $skipTest = false;
+
 	/**
 	* Get details on each of the external storage backends, used for the mount config UI
 	* If a custom UI is needed, add the key 'custom' and a javascript file with that name will be loaded
@@ -39,6 +49,7 @@ class OC_Mount_Config {
 	*/
 	public static function getBackends() {
 
+		// FIXME: do not rely on php key order for the options order in the UI
 		$backends['\OC\Files\Storage\Local']=array(
 				'backend' => 'Local',
 				'configuration' => array(
@@ -146,6 +157,125 @@ class OC_Mount_Config {
 	}
 
 	/**
+	 * Hook that mounts the given user's visible mount points
+	 * @param array $data
+	 */
+	public static function initMountPointsHook($data) {
+		$mountPoints = self::getAbsoluteMountPoints($data['user']);
+		foreach ($mountPoints as $mountPoint => $options) {
+			\OC\Files\Filesystem::mount($options['class'], $options['options'], $mountPoint);
+		}
+	}
+
+	/**
+	 * Returns the mount points for the given user.
+	 * The mount point is relative to the data directory.
+	 *
+	 * @param string $user user
+	 * @return array of mount point string as key, mountpoint config as value
+	 */
+	public static function getAbsoluteMountPoints($user) {
+		$mountPoints = array();	
+
+		$datadir = \OC_Config::getValue("datadirectory", \OC::$SERVERROOT . "/data");
+		$mount_file = \OC_Config::getValue("mount_file", $datadir . "/mount.json");
+
+		//move config file to it's new position
+		if (is_file(\OC::$SERVERROOT . '/config/mount.json')) {
+			rename(\OC::$SERVERROOT . '/config/mount.json', $mount_file);
+		}
+
+		// Load system mount points
+		$mountConfig = self::readData(false);
+		if (isset($mountConfig[self::MOUNT_TYPE_GLOBAL])) {
+			foreach ($mountConfig[self::MOUNT_TYPE_GLOBAL] as $mountPoint => $options) {
+				$options['options'] = self::decryptPasswords($options['options']);
+				$mountPoints[$mountPoint] = $options;
+			}
+		}
+		if (isset($mountConfig[self::MOUNT_TYPE_GROUP])) {
+			foreach ($mountConfig[self::MOUNT_TYPE_GROUP] as $group => $mounts) {
+				if (\OC_Group::inGroup($user, $group)) {
+					foreach ($mounts as $mountPoint => $options) {
+						$mountPoint = self::setUserVars($user, $mountPoint);
+						foreach ($options as &$option) {
+							$option = self::setUserVars($user, $option);
+						}
+						$options['options'] = self::decryptPasswords($options['options']);
+						$mountPoints[$mountPoint] = $options;
+					}
+				}
+			}
+		}
+		if (isset($mountConfig[self::MOUNT_TYPE_USER])) {
+			foreach ($mountConfig[self::MOUNT_TYPE_USER] as $mountUser => $mounts) {
+				if ($mountUser === 'all' or strtolower($mountUser) === strtolower($user)) {
+					foreach ($mounts as $mountPoint => $options) {
+						$mountPoint = self::setUserVars($user, $mountPoint);
+						foreach ($options as &$option) {
+							$option = self::setUserVars($user, $option);
+						}
+						$options['options'] = self::decryptPasswords($options['options']);
+						$mountPoints[$mountPoint] = $options;
+					}
+				}
+			}
+		}
+
+		// Load personal mount points
+		$mountConfig = self::readData(true);
+		if (isset($mountConfig[self::MOUNT_TYPE_USER][$user])) {
+			foreach ($mountConfig[self::MOUNT_TYPE_USER][$user] as $mountPoint => $options) {
+				$options['options'] = self::decryptPasswords($options['options']);
+				$mountPoints[$mountPoint] = $options;
+			}
+		}
+
+		return $mountPoints;
+	}
+
+	/**
+	 * fill in the correct values for $user
+	 *
+	 * @param string $user
+	 * @param string $input
+	 * @return string
+	 */
+	private static function setUserVars($user, $input) {
+		return str_replace('$user', $user, $input);
+	}
+
+
+	/**
+	* Get details on each of the external storage backends, used for the mount config UI
+	* Some backends are not available as a personal backend, f.e. Local and such that have
+	* been disabled by the admin.
+	*
+	* If a custom UI is needed, add the key 'custom' and a javascript file with that name will be loaded
+	* If the configuration parameter should be secret, add a '*' to the beginning of the value
+	* If the configuration parameter is a boolean, add a '!' to the beginning of the value
+	* If the configuration parameter is optional, add a '&' to the beginning of the value
+	* If the configuration parameter is hidden, add a '#' to the beginning of the value
+	* @return array
+	*/
+	public static function getPersonalBackends() {
+
+		$backends = self::getBackends();
+
+		// Remove local storage and other disabled storages
+		unset($backends['\OC\Files\Storage\Local']);
+
+		$allowed_backends = explode(',', OCP\Config::getAppValue('files_external', 'user_mounting_backends', ''));
+		foreach ($backends as $backend => $null) {
+			if (!in_array($backend, $allowed_backends)) {
+				unset($backends[$backend]);
+			}
+		}
+
+		return $backends;
+	}
+
+	/**
 	* Get the system mount points
 	* The returned array is not in the same format as getUserMountPoints()
 	* @return array
@@ -161,6 +291,7 @@ class OC_Mount_Config {
 					if (strpos($mount['class'], 'OC_Filestorage_') !== false) {
 						$mount['class'] = '\OC\Files\Storage\\'.substr($mount['class'], 15);
 					}
+					$mount['options'] = self::decryptPasswords($mount['options']);
 					// Remove '/$user/files/' from mount point
 					$mountPoint = substr($mountPoint, 13);
 					// Merge the mount point into the current mount points
@@ -186,6 +317,7 @@ class OC_Mount_Config {
 					if (strpos($mount['class'], 'OC_Filestorage_') !== false) {
 						$mount['class'] = '\OC\Files\Storage\\'.substr($mount['class'], 15);
 					}
+					$mount['options'] = self::decryptPasswords($mount['options']);
 					// Remove '/$user/files/' from mount point
 					$mountPoint = substr($mountPoint, 13);
 					// Merge the mount point into the current mount points
@@ -223,6 +355,7 @@ class OC_Mount_Config {
 				if (strpos($mount['class'], 'OC_Filestorage_') !== false) {
 					$mount['class'] = '\OC\Files\Storage\\'.substr($mount['class'], 15);
 				}
+				$mount['options'] = self::decryptPasswords($mount['options']);
 				// Remove '/uid/files/' from mount point
 				$personal[substr($mountPoint, strlen($uid) + 8)] = array(
 					'class' => $mount['class'],
@@ -235,9 +368,18 @@ class OC_Mount_Config {
 		return $personal;
 	}
 
+	/**
+	 * Test connecting using the given backend configuration
+	 * @param string $class backend class name
+	 * @param array $options backend configuration options
+	 * @return bool true if the connection succeeded, false otherwise
+	 */
 	private static function getBackendStatus($class, $options) {
+		if (self::$skipTest) {
+			return true;
+		}
 		foreach ($options as &$option) {
-			$option = str_replace('$user', OCP\User::getUser(), $option);
+			$option = self::setUserVars(OCP\User::getUser(), $option);
 		}
 		if (class_exists($class)) {
 			try {
@@ -288,7 +430,13 @@ class OC_Mount_Config {
 		} else {
 			$mountPoint = '/$user/files/'.ltrim($mountPoint, '/');
 		}
-		$mount = array($applicable => array($mountPoint => array('class' => $class, 'options' => $classOptions)));
+
+		$mount = array($applicable => array(
+			$mountPoint => array(
+				'class' => $class,
+				'options' => self::encryptPasswords($classOptions))
+			)
+		);
 		$mountPoints = self::readData($isPersonal);
 		// Merge the new mount point into the current mount points
 		if (isset($mountPoints[$mountType])) {
@@ -480,4 +628,71 @@ class OC_Mount_Config {
 
 		return $txt;
 	}
+
+	/**
+	 * Encrypt passwords in the given config options
+	 * @param array $options mount options
+	 * @return array updated options
+	 */
+	private static function encryptPasswords($options) {
+		if (isset($options['password'])) {
+			$options['password_encrypted'] = self::encryptPassword($options['password']);
+			// do not unset the password, we want to keep the keys order
+			// on load... because that's how the UI currently works
+			$options['password'] = '';
+		}
+		return $options;
+	}
+
+	/**
+	 * Decrypt passwords in the given config options
+	 * @param array $options mount options
+	 * @return array updated options
+	 */
+	private static function decryptPasswords($options) {
+		// note: legacy options might still have the unencrypted password in the "password" field
+		if (isset($options['password_encrypted'])) {
+			$options['password'] = self::decryptPassword($options['password_encrypted']);
+			unset($options['password_encrypted']);
+		}
+		return $options;
+	}
+
+	/**
+	 * Encrypt a single password
+	 * @param string $password plain text password
+	 * @return encrypted password
+	 */
+	private static function encryptPassword($password) {
+		$cipher = self::getCipher();
+		$iv = \OCP\Util::generateRandomBytes(16);
+		$cipher->setIV($iv);
+		return base64_encode($iv . $cipher->encrypt($password));
+	}
+
+	/**
+	 * Decrypts a single password
+	 * @param string $encryptedPassword encrypted password
+	 * @return plain text password
+	 */
+	private static function decryptPassword($encryptedPassword) {
+		$cipher = self::getCipher();
+		$binaryPassword = base64_decode($encryptedPassword);
+		$iv = substr($binaryPassword, 0, 16);
+		$cipher->setIV($iv);
+		$binaryPassword = substr($binaryPassword, 16);
+		return $cipher->decrypt($binaryPassword);
+	}
+
+	/**
+	 * Returns the encryption cipher
+	 */
+	private static function getCipher() {
+		if (!class_exists('Crypt_AES', false)) {
+			include('Crypt/AES.php');
+		}
+		$cipher = new Crypt_AES(CRYPT_AES_MODE_CBC);
+		$cipher->setKey(\OCP\Config::getSystemValue('passwordsalt'));
+		return $cipher;
+	}
 }
diff --git a/apps/files_external/lib/smb.php b/apps/files_external/lib/smb.php
index c5fba92ee68..f3f3b3ed7f3 100644
--- a/apps/files_external/lib/smb.php
+++ b/apps/files_external/lib/smb.php
@@ -37,7 +37,7 @@ class SMB extends \OC\Files\Storage\StreamWrapper{
 				$this->share = substr($this->share, 0, -1);
 			}
 		} else {
-			throw new \Exception();
+			throw new \Exception('Invalid configuration');
 		}
 	}
 
diff --git a/apps/files_external/tests/mountconfig.php b/apps/files_external/tests/mountconfig.php
index 24ebcf51346..0a0bb7685d1 100644
--- a/apps/files_external/tests/mountconfig.php
+++ b/apps/files_external/tests/mountconfig.php
@@ -34,6 +34,90 @@ class Test_Mount_Config_Dummy_Storage {
  * Class Test_Mount_Config
  */
 class Test_Mount_Config extends \PHPUnit_Framework_TestCase {
+
+	private $dataDir;
+	private $userHome;
+	private $oldAllowedBackends;
+	private $allBackends;
+
+	const TEST_USER1 = 'user1';
+	const TEST_USER2 = 'user2';
+	const TEST_GROUP1 = 'group1';
+	const TEST_GROUP2 = 'group2';
+
+	public function setUp() {
+		\OC_User::createUser(self::TEST_USER1, self::TEST_USER1);
+		\OC_User::createUser(self::TEST_USER2, self::TEST_USER2);
+
+		\OC_Group::createGroup(self::TEST_GROUP1);
+		\OC_Group::addToGroup(self::TEST_USER1, self::TEST_GROUP1);
+		\OC_Group::createGroup(self::TEST_GROUP2);
+		\OC_Group::addToGroup(self::TEST_USER2, self::TEST_GROUP2);
+
+		\OC_User::setUserId(self::TEST_USER1);
+		$this->userHome = \OC_User::getHome(self::TEST_USER1);
+		mkdir($this->userHome);
+
+		$this->dataDir = \OC_Config::getValue(
+			'datadirectory',
+			\OC::$SERVERROOT . '/data/'
+		);
+		$this->oldAllowedBackends = OCP\Config::getAppValue(
+			'files_external',
+			'user_mounting_backends',
+			''
+		);
+		$this->allBackends = OC_Mount_Config::getBackends();
+		OCP\Config::setAppValue(
+			'files_external',
+			'user_mounting_backends',
+			implode(',', array_keys($this->allBackends))
+		);
+
+		OC_Mount_Config::$skipTest = true;
+	}
+
+	public function tearDown() {
+		OC_Mount_Config::$skipTest = false;
+
+		\OC_User::deleteUser(self::TEST_USER2);
+		\OC_User::deleteUser(self::TEST_USER1);
+		\OC_Group::deleteGroup(self::TEST_GROUP1);
+		\OC_Group::deleteGroup(self::TEST_GROUP2);
+
+		@unlink($this->dataDir . '/mount.json');
+
+		OCP\Config::setAppValue(
+			'files_external',
+			'user_mounting_backends',
+			$this->oldAllowedBackends
+		);
+	}
+
+	/**
+	 * Reads the global config, for checking
+	 */
+	private function readGlobalConfig() {
+		$configFile = $this->dataDir . '/mount.json';
+		return json_decode(file_get_contents($configFile), true);
+	}
+
+	/**
+	 * Reads the user config, for checking
+	 */
+	private function readUserConfig() {
+		$configFile = $this->userHome . '/mount.json';
+		return json_decode(file_get_contents($configFile), true);
+	}
+
+	/**
+	 * Write the user config, to simulate existing files
+	 */
+	private function writeUserConfig($config) {
+		$configFile = $this->userHome . '/mount.json';
+		file_put_contents($configFile, json_encode($config));
+	}
+
 	/**
 	 * Test mount point validation
 	 */
@@ -42,35 +126,329 @@ class Test_Mount_Config extends \PHPUnit_Framework_TestCase {
 		$mountType = 'user';
 		$applicable = 'all';
 		$isPersonal = false;
-		$this->assertEquals(false, OC_Mount_Config::addMountPoint('', $storageClass, array(), $mountType, $applicable, $isPersonal));
-		$this->assertEquals(false, OC_Mount_Config::addMountPoint('/', $storageClass, array(), $mountType, $applicable, $isPersonal));
-		$this->assertEquals(false, OC_Mount_Config::addMountPoint('Shared', $storageClass, array(), $mountType, $applicable, $isPersonal));
-		$this->assertEquals(false, OC_Mount_Config::addMountPoint('/Shared', $storageClass, array(), $mountType, $applicable, $isPersonal));
+		$this->assertFalse(OC_Mount_Config::addMountPoint('', $storageClass, array(), $mountType, $applicable, $isPersonal));
+		$this->assertFalse(OC_Mount_Config::addMountPoint('/', $storageClass, array(), $mountType, $applicable, $isPersonal));
+		$this->assertFalse(OC_Mount_Config::addMountPoint('Shared', $storageClass, array(), $mountType, $applicable, $isPersonal));
+		$this->assertFalse(OC_Mount_Config::addMountPoint('/Shared', $storageClass, array(), $mountType, $applicable, $isPersonal));
 
 	}
 
+	/**
+	 * Test adding a global mount point
+	 */
+	public function testAddGlobalMountPoint() {
+		$mountType = OC_Mount_Config::MOUNT_TYPE_USER;
+		$applicable = 'all';
+		$isPersonal = false;
+
+		$this->assertEquals(true, OC_Mount_Config::addMountPoint('/ext', '\OC\Files\Storage\SFTP', array(), $mountType, $applicable, $isPersonal));
+
+		$config = $this->readGlobalConfig();
+		$this->assertEquals(1, count($config));
+		$this->assertTrue(isset($config[$mountType]));
+		$this->assertTrue(isset($config[$mountType][$applicable]));
+		$this->assertTrue(isset($config[$mountType][$applicable]['/$user/files/ext']));
+		$this->assertEquals(
+			'\OC\Files\Storage\SFTP',
+			$config[$mountType][$applicable]['/$user/files/ext']['class']
+		);
+	}
+
+	/**
+	 * Test adding a personal mount point
+	 */
 	public function testAddMountPointSingleUser() {
-		\OC_User::setUserId('test');
-		$mountType = 'user';
-		$applicable = 'test';
+		$mountType = OC_Mount_Config::MOUNT_TYPE_USER;
+		$applicable = self::TEST_USER1;
 		$isPersonal = true;
-		// local
-		$this->assertEquals(false, OC_Mount_Config::addMountPoint('/ext', '\OC\Files\storage\local', array(), $mountType, $applicable, $isPersonal));
-		// non-local
-		// FIXME: can't test this yet as the class (write operation) is not mockable
-		// $this->assertEquals(true, OC_Mount_Config::addMountPoint('/ext', '\OC\Files\Storage\SFTP', array(), $mountType, $applicable, $isPersonal));
 
+		$this->assertEquals(true, OC_Mount_Config::addMountPoint('/ext', '\OC\Files\Storage\SFTP', array(), $mountType, $applicable, $isPersonal));
+
+		$config = $this->readUserConfig();
+		$this->assertEquals(1, count($config));
+		$this->assertTrue(isset($config[$mountType]));
+		$this->assertTrue(isset($config[$mountType][$applicable]));
+		$this->assertTrue(isset($config[$mountType][$applicable]['/' . self::TEST_USER1 . '/files/ext']));
+		$this->assertEquals(
+			'\OC\Files\Storage\SFTP',
+			$config[$mountType][$applicable]['/' . self::TEST_USER1 . '/files/ext']['class']
+		);
 	}
 
+	/**
+	 * Test adding a mount point with an non-existant backend
+	 */
 	public function testAddMountPointUnexistClass() {
-		\OC_User::setUserId('test');
 		$storageClass = 'Unexist_Storage';
-		$mountType = 'user';
-		$applicable = 'test';
+		$mountType = OC_Mount_Config::MOUNT_TYPE_USER;
+		$applicable = self::TEST_USER1;
+		$isPersonal = false;
+		$this->assertFalse(OC_Mount_Config::addMountPoint('/ext', $storageClass, array(), $mountType, $applicable, $isPersonal));
+
+	}
+
+	/**
+	 * Test reading and writing global config
+	 */
+	public function testReadWriteGlobalConfig() {
+		$mountType = OC_Mount_Config::MOUNT_TYPE_USER;
+		$applicable = 'all';
+		$isPersonal = false;
+		$mountConfig = array(
+			'host' => 'smbhost',
+			'user' => 'smbuser',
+			'password' => 'smbpassword',
+			'share' => 'smbshare',
+			'root' => 'smbroot'
+		);
+
+		// write config
+		$this->assertTrue(
+			OC_Mount_Config::addMountPoint(
+				'/ext',
+				'\OC\Files\Storage\SMB',
+				$mountConfig,
+				$mountType,
+				$applicable,
+				$isPersonal
+			)
+		);
+
+		// re-read config
+		$config = OC_Mount_Config::getSystemMountPoints();
+		$this->assertEquals(1, count($config));
+		$this->assertTrue(isset($config['ext']));
+		$this->assertEquals('\OC\Files\Storage\SMB', $config['ext']['class']);
+		$savedMountConfig = $config['ext']['configuration'];
+		$this->assertEquals($mountConfig, $savedMountConfig);
+		// key order needs to be preserved for the UI...
+		$this->assertEquals(array_keys($mountConfig), array_keys($savedMountConfig));
+	}
+
+	/**
+	 * Test reading and writing config
+	 */
+	public function testReadWritePersonalConfig() {
+		$mountType = OC_Mount_Config::MOUNT_TYPE_USER;
+		$applicable = self::TEST_USER1;
+		$isPersonal = true;
+		$mountConfig = array(
+			'host' => 'smbhost',
+			'user' => 'smbuser',
+			'password' => 'smbpassword',
+			'share' => 'smbshare',
+			'root' => 'smbroot'
+		);
+
+		// write config
+		$this->assertTrue(
+			OC_Mount_Config::addMountPoint(
+				'/ext',
+				'\OC\Files\Storage\SMB',
+				$mountConfig,
+				$mountType,
+				$applicable,
+				$isPersonal
+			)
+		);
+
+		// re-read config
+		$config = OC_Mount_Config::getPersonalMountPoints();
+		$this->assertEquals(1, count($config));
+		$this->assertTrue(isset($config['ext']));
+		$this->assertEquals('\OC\Files\Storage\SMB', $config['ext']['class']);
+		$savedMountConfig = $config['ext']['configuration'];
+		$this->assertEquals($mountConfig, $savedMountConfig);
+		// key order needs to be preserved for the UI...
+		$this->assertEquals(array_keys($mountConfig), array_keys($savedMountConfig));
+	}
+
+	/**
+	 * Test password obfuscation
+	 */
+	public function testPasswordObfuscation() {
+		$mountType = OC_Mount_Config::MOUNT_TYPE_USER;
+		$applicable = self::TEST_USER1;
+		$isPersonal = true;
+		$mountConfig = array(
+			'host' => 'smbhost',
+			'user' => 'smbuser',
+			'password' => 'smbpassword',
+			'share' => 'smbshare',
+			'root' => 'smbroot'
+		);
+
+		// write config
+		$this->assertTrue(
+			OC_Mount_Config::addMountPoint(
+				'/ext',
+				'\OC\Files\Storage\SMB',
+				$mountConfig,
+				$mountType,
+				$applicable,
+				$isPersonal
+			)
+		);
+
+		// note: password re-reading is covered by testReadWritePersonalConfig
+
+		// check that password inside the file is NOT in plain text
+		$config = $this->readUserConfig();
+		$savedConfig = $config[$mountType][$applicable]['/' . self::TEST_USER1 . '/files/ext']['options'];
+
+		// no more clear text password in file (kept because of key order)
+		$this->assertEquals('', $savedConfig['password']);
+
+		// encrypted password is present
+		$this->assertNotEquals($mountConfig['password'], $savedConfig['password_encrypted']);
+	}
+
+	/**
+	 * Test read legacy passwords
+	 */
+	public function testReadLegacyPassword() {
+		$mountType = OC_Mount_Config::MOUNT_TYPE_USER;
+		$applicable = self::TEST_USER1;
 		$isPersonal = true;
-		// local
-		// non-local
-		$this->assertEquals(false, OC_Mount_Config::addMountPoint('/ext', $storageClass, array(), $mountType, $applicable, $isPersonal));
+		$mountConfig = array(
+			'host' => 'smbhost',
+			'user' => 'smbuser',
+			'password' => 'smbpassword',
+			'share' => 'smbshare',
+			'root' => 'smbroot'
+		);
+
+		// write config
+		$this->assertTrue(
+			OC_Mount_Config::addMountPoint(
+				'/ext',
+				'\OC\Files\Storage\SMB',
+				$mountConfig,
+				$mountType,
+				$applicable,
+				$isPersonal
+			)
+		);
+
+		$config = $this->readUserConfig();
+		// simulate non-encrypted password situation
+		$config[$mountType][$applicable]['/' . self::TEST_USER1 . '/files/ext']['options']['password'] = 'smbpasswd';
+
+		$this->writeUserConfig($config);
+
+		// re-read config, password was read correctly
+		$config = OC_Mount_Config::getPersonalMountPoints();
+		$savedMountConfig = $config['ext']['configuration'];
+		$this->assertEquals($mountConfig, $savedMountConfig);
+	}
+
+	public function mountDataProvider() {
+		return array(
+			// Tests for visible mount points
+			// system mount point for all users
+			array(
+				false,
+				OC_Mount_Config::MOUNT_TYPE_USER,
+				'all',
+				self::TEST_USER1,
+				true,
+			),
+			// system mount point for a specific user
+			array(
+				false,
+				OC_Mount_Config::MOUNT_TYPE_USER,
+				self::TEST_USER1,
+				self::TEST_USER1,
+				true,
+			),
+			// system mount point for a specific group
+			array(
+				false,
+				OC_Mount_Config::MOUNT_TYPE_GROUP,
+				self::TEST_GROUP1,
+				self::TEST_USER1,
+				true,
+			),
+			// user mount point
+			array(
+				true,
+				OC_Mount_Config::MOUNT_TYPE_USER,
+				self::TEST_USER1,
+				self::TEST_USER1,
+				true,
+			),
+
+			// Tests for non-visible mount points
+			// system mount point for another user
+			array(
+				false,
+				OC_Mount_Config::MOUNT_TYPE_USER,
+				self::TEST_USER2,
+				self::TEST_USER1,
+				false,
+			),
+			// system mount point for a specific group
+			array(
+				false,
+				OC_Mount_Config::MOUNT_TYPE_GROUP,
+				self::TEST_GROUP2,
+				self::TEST_USER1,
+				false,
+			),
+			// user mount point
+			array(
+				true,
+				OC_Mount_Config::MOUNT_TYPE_USER,
+				self::TEST_USER1,
+				self::TEST_USER2,
+				false,
+			),
+		);
+	}
+
+	/**
+	 * Test mount points used at mount time, making sure
+	 * the configuration is prepared properly.
+	 *
+	 * @dataProvider mountDataProvider
+	 * @param bool $isPersonal true for personal mount point, false for system mount point
+	 * @param string $mountType mount type
+	 * @param string $applicable target user/group or "all"
+	 * @param string $testUser user for which to retrieve the mount points
+	 * @param bool $expectVisible whether to expect the mount point to be visible for $testUser
+	 */
+	public function testMount($isPersonal, $mountType, $applicable, $testUser, $expectVisible) {
+		$mountConfig = array(
+			'host' => 'someost',
+			'user' => 'someuser',
+			'password' => 'somepassword',
+			'root' => 'someroot'
+		);
+
+		// add mount point as "test" user 
+		$this->assertTrue(
+			OC_Mount_Config::addMountPoint(
+				'/ext',
+				'\OC\Files\Storage\SMB',
+				$mountConfig,
+				$mountType,
+				$applicable,
+				$isPersonal
+			)
+		);
+
+		// check mount points in the perspective of user $testUser
+		\OC_User::setUserId($testUser);
 
+		$mountPoints = OC_Mount_Config::getAbsoluteMountPoints($testUser);
+		if ($expectVisible) {
+			$this->assertEquals(1, count($mountPoints));
+			$this->assertTrue(isset($mountPoints['/' . self::TEST_USER1 . '/files/ext']));
+			$this->assertEquals('\OC\Files\Storage\SMB', $mountPoints['/' . self::TEST_USER1 . '/files/ext']['class']);
+			$this->assertEquals($mountConfig, $mountPoints['/' . self::TEST_USER1 . '/files/ext']['options']);
+		}
+		else {
+			$this->assertEquals(0, count($mountPoints));
+		}
 	}
 }