Merge pull request #3023 from nextcloud/issue-2915-filter-out-sensitive-appconfigs

Filter out sensitive appconfig values

core/Command/Config/ListConfigs.php

@@ -89,14 +89,14 @@ class ListConfigs extends Base {
 					'apps' => [],
 				];
 				foreach ($apps as $appName) {
-					$configs['apps'][$appName] = $this->appConfig->getValues($appName, false);
+					$configs['apps'][$appName] = $this->getAppConfigs($appName, $noSensitiveValues);
 				}
 			break;
 
 			default:
 				$configs = [
 					'apps' => [
-						$app => $this->appConfig->getValues($app, false),
+						$app => $this->getAppConfigs($app, $noSensitiveValues),
 					],
 				];
 		}
@@ -129,6 +129,21 @@ class ListConfigs extends Base {
 		return $configs;
 	}
 
+	/**
+	 * Get the app configs
+	 *
+	 * @param string $app
+	 * @param bool $noSensitiveValues
+	 * @return array
+	 */
+	protected function getAppConfigs($app, $noSensitiveValues) {
+		if ($noSensitiveValues) {
+			return $this->appConfig->getFilteredValues($app, false);
+		} else {
+			return $this->appConfig->getValues($app, false);
+		}
+	}
+
 	/**
 	 * @param string $argumentName
 	 * @param CompletionContext $context

lib/private/AllConfig.php

@@ -68,7 +68,7 @@ class AllConfig implements \OCP\IConfig {
 	/**
 	 * @param SystemConfig $systemConfig
 	 */
-	function __construct(SystemConfig $systemConfig) {
+	public function __construct(SystemConfig $systemConfig) {
 		$this->userCache = new CappedMemoryCache();
 		$this->systemConfig = $systemConfig;
 	}

lib/private/AppConfig.php

@@ -29,7 +29,9 @@
 
 namespace OC;
 
+use OC\DB\OracleConnection;
 use OCP\IAppConfig;
+use OCP\IConfig;
 use OCP\IDBConnection;
 
 /**
@@ -37,12 +39,22 @@ use OCP\IDBConnection;
  * database.
  */
 class AppConfig implements IAppConfig {
-	/**
-	 * @var \OCP\IDBConnection $conn
-	 */
+
+	/** @var array[] */
+	protected $sensitiveValues = [
+		'user_ldap' => [
+			'ldap_agent_password',
+		],
+	];
+
+	/** @var \OCP\IDBConnection */
 	protected $conn;
 
-	private $cache = array();
+	/** @var array[] */
+	private $cache = [];
+
+	/** @var bool */
+	private $configLoaded = false;
 
 	/**
 	 * @param IDBConnection $conn
@@ -85,6 +97,7 @@ class AppConfig implements IAppConfig {
 	 *
 	 * @param string $app the app we are looking for
 	 * @return array an array of key names
+	 * @deprecated 8.0.0 use method getAppKeys of \OCP\IConfig
 	 *
 	 * This function gets all keys of an app. Please note that the values are
 	 * not returned.
@@ -112,6 +125,7 @@ class AppConfig implements IAppConfig {
 	 * @param string $key key
 	 * @param string $default = null, default value if the key does not exist
 	 * @return string the value or $default
+	 * @deprecated 8.0.0 use method getAppValue of \OCP\IConfig
 	 *
 	 * This function gets a value from the appconfig table. If the key does
 	 * not exist the default value will be returned
@@ -146,6 +160,7 @@ class AppConfig implements IAppConfig {
 	 * @param string $key key
 	 * @param string|float|int $value value
 	 * @return bool True if the value was inserted or updated, false if the value was the same
+	 * @deprecated 8.0.0 use method setAppValue of \OCP\IConfig
 	 */
 	public function setValue($app, $key, $value) {
 		if (!$this->hasKey($app, $key)) {
@@ -182,7 +197,7 @@ class AppConfig implements IAppConfig {
 		 * http://docs.oracle.com/cd/E11882_01/server.112/e26088/conditions002.htm#i1033286
 		 * > Large objects (LOBs) are not supported in comparison conditions.
 		 */
-		if (!($this->conn instanceof \OC\DB\OracleConnection)) {
+		if (!($this->conn instanceof OracleConnection)) {
 			// Only update the value when it is not the same
 			$sql->andWhere($sql->expr()->neq('configvalue', $sql->createParameter('configvalue')))
 				->setParameter('configvalue', $value);
@@ -200,7 +215,8 @@ class AppConfig implements IAppConfig {
 	 *
 	 * @param string $app app
 	 * @param string $key key
-	 * @return boolean|null
+	 * @return boolean
+	 * @deprecated 8.0.0 use method deleteAppValue of \OCP\IConfig
 	 */
 	public function deleteKey($app, $key) {
 		$this->loadConfigValues();
@@ -214,13 +230,15 @@ class AppConfig implements IAppConfig {
 		$sql->execute();
 
 		unset($this->cache[$app][$key]);
+		return false;
 	}
 
 	/**
 	 * Remove app from appconfig
 	 *
 	 * @param string $app app
-	 * @return boolean|null
+	 * @return boolean
+	 * @deprecated 8.0.0 use method deleteAppValue of \OCP\IConfig
 	 *
 	 * Removes all keys in appconfig belonging to the app.
 	 */
@@ -234,6 +252,7 @@ class AppConfig implements IAppConfig {
 		$sql->execute();
 
 		unset($this->cache[$app]);
+		return false;
 	}
 
 	/**
@@ -261,11 +280,31 @@ class AppConfig implements IAppConfig {
 		}
 	}
 
+	/**
+	 * get all values of the app or and filters out sensitive data
+	 *
+	 * @param string $app
+	 * @return array
+	 */
+	public function getFilteredValues($app) {
+		$values = $this->getValues($app, false);
+
+		foreach ($this->sensitiveValues[$app] as $sensitiveKey) {
+			if (isset($values[$sensitiveKey])) {
+				$values[$sensitiveKey] = IConfig::SENSITIVE_VALUE;
+			}
+		}
+
+		return $values;
+	}
+
 	/**
 	 * Load all the app config values
 	 */
 	protected function loadConfigValues() {
-		if ($this->configLoaded) return;
+		if ($this->configLoaded) {
+			return;
+		}
 
 		$this->cache = [];
 

lib/private/SystemConfig.php

@@ -44,7 +44,6 @@ class SystemConfig {
 		'passwordsalt' => true,
 		'secret' => true,
 		'updater.secret' => true,
-		'ldap_agent_password' => true,
 		'proxyuserpwd' => true,
 		'log.condition' => [
 			'shared_secret' => true,

lib/public/IAppConfig.php

@@ -86,6 +86,15 @@ interface IAppConfig {
 	 */
 	public function getValues($app, $key);
 
+	/**
+	 * get all values of the app or and filters out sensitive data
+	 *
+	 * @param string $app
+	 * @return array
+	 * @since 12.0.0
+	 */
+	public function getFilteredValues($app);
+
 	/**
 	 * sets a value in the appconfig
 	 * @param string $app app

tests/Core/Command/Config/ListConfigsTest.php

@@ -285,10 +285,16 @@ class ListConfigsTest extends TestCase {
 			$this->systemConfig->expects($this->any())
 				->method('getValue')
 				->willReturnMap($systemConfigMap);
+			$this->appConfig->expects($this->any())
+				->method('getValues')
+				->willReturnMap($appConfig);
 		} else {
 			$this->systemConfig->expects($this->any())
 				->method('getFilteredValue')
 				->willReturnMap($systemConfigMap);
+			$this->appConfig->expects($this->any())
+				->method('getFilteredValues')
+				->willReturnMap($appConfig);
 		}
 
 		$this->appConfig->expects($this->any())

tests/lib/AppConfigTest.php

@@ -8,6 +8,7 @@
  */
 
 namespace Test;
+use OCP\IConfig;
 
 /**
  * Class AppConfigTest
@@ -305,6 +306,28 @@ class AppConfigTest extends TestCase {
 		$this->assertEquals($expected, $values);
 	}
 
+	public function testGetFilteredValues() {
+		/** @var \OC\AppConfig|\PHPUnit_Framework_MockObject_MockObject $config */
+		$config = $this->getMockBuilder(\OC\AppConfig::class)
+			->setConstructorArgs([\OC::$server->getDatabaseConnection()])
+			->setMethods(['getValues'])
+			->getMock();
+
+		$config->expects($this->once())
+			->method('getValues')
+			->with('user_ldap', false)
+			->willReturn([
+				'ldap_agent_password' => 'secret',
+				'ldap_dn' => 'dn',
+			]);
+
+		$values = $config->getFilteredValues('user_ldap');
+		$this->assertEquals([
+			'ldap_agent_password' => IConfig::SENSITIVE_VALUE,
+			'ldap_dn' => 'dn',
+		], $values);
+	}
+
 	public function testSettingConfigParallel() {
 		$appConfig1 = new \OC\AppConfig(\OC::$server->getDatabaseConnection());
 		$appConfig2 = new \OC\AppConfig(\OC::$server->getDatabaseConnection());