Move the filtering of sensitive data to the config class

5 files changed, 99 insertions, 48 deletions

diff --git a/core/command/config/listconfigs.php b/core/command/config/listconfigs.php
index 5796362f2fc..37aeb53c6f5 100644
--- a/core/command/config/listconfigs.php
+++ b/core/command/config/listconfigs.php
@@ -32,20 +32,6 @@ use Symfony\Component\Console\Output\OutputInterface;
 class ListConfigs extends Base {
 	protected $defaultOutputFormat = self::OUTPUT_FORMAT_JSON_PRETTY;
 
-	/** @var array */
-	protected $sensitiveValues = [
-		'dbpassword' => true,
-		'dbuser' => true,
-		'mail_smtpname' => true,
-		'mail_smtppassword' => true,
-		'passwordsalt' => true,
-		'secret' => true,
-		'ldap_agent_password' => true,
-		'objectstore' => ['arguments' => ['password' => true]],
-	];
-
-	const SENSITIVE_VALUE = '***REMOVED SENSITIVE VALUE***';
-
 	/** * @var SystemConfig */
 	protected $systemConfig;
 
@@ -127,10 +113,10 @@ class ListConfigs extends Base {
 
 		$configs = [];
 		foreach ($keys as $key) {
-			$value = $this->systemConfig->getValue($key, serialize(null));
-
-			if ($noSensitiveValues && isset($this->sensitiveValues[$key])) {
-				$value = $this->removeSensitiveValue($this->sensitiveValues[$key], $value);
+			if ($noSensitiveValues) {
+				$value = $this->systemConfig->getFilteredValue($key, serialize(null));
+			} else {
+				$value = $this->systemConfig->getValue($key, serialize(null));
 			}
 
 			if ($value !== 'N;') {
@@ -140,25 +126,4 @@ class ListConfigs extends Base {
 
 		return $configs;
 	}
-
-	/**
-	 * @param bool|array $keysToRemove
-	 * @param mixed $value
-	 * @return mixed
-	 */
-	protected function removeSensitiveValue($keysToRemove, $value) {
-		if ($keysToRemove === true) {
-			return self::SENSITIVE_VALUE;
-		}
-
-		if (is_array($value)) {
-			foreach ($keysToRemove as $keyToRemove => $valueToRemove) {
-				if (isset($value[$keyToRemove])) {
-					$value[$keyToRemove] = $this->removeSensitiveValue($valueToRemove, $value[$keyToRemove]);
-				}
-			}
-		}
-
-		return $value;
-	}
 }
diff --git a/lib/private/allconfig.php b/lib/private/allconfig.php
index 63cc92601bb..7c2037e8048 100644
--- a/lib/private/allconfig.php
+++ b/lib/private/allconfig.php
@@ -119,6 +119,17 @@ class AllConfig implements \OCP\IConfig {
 	}
 
 	/**
+	 * Looks up a system wide defined value and filters out sensitive data
+	 *
+	 * @param string $key the key of the value, under which it was saved
+	 * @param mixed $default the default value to be returned if the value isn't set
+	 * @return mixed the value or $default
+	 */
+	public function getFilteredSystemValue($key, $default = '') {
+		return $this->systemConfig->getFilteredValue($key, $default);
+	}
+
+	/**
 	 * Delete a system wide defined value
 	 *
 	 * @param string $key the key of the value, under which it was saved
diff --git a/lib/private/systemconfig.php b/lib/private/systemconfig.php
index 13b0959768a..3b7930f2842 100644
--- a/lib/private/systemconfig.php
+++ b/lib/private/systemconfig.php
@@ -28,6 +28,21 @@ namespace OC;
  * fixes cyclic DI: AllConfig needs AppConfig needs Database needs AllConfig
  */
 class SystemConfig {
+
+	/** @var array */
+	protected $sensitiveValues = [
+		'dbpassword' => true,
+		'dbuser' => true,
+		'mail_smtpname' => true,
+		'mail_smtppassword' => true,
+		'passwordsalt' => true,
+		'secret' => true,
+		'ldap_agent_password' => true,
+		'objectstore' => ['arguments' => ['password' => true]],
+	];
+
+	const SENSITIVE_VALUE = '***REMOVED SENSITIVE VALUE***';
+
 	/**
 	 * Lists all available config keys
 	 * @return array an array of key names
@@ -68,6 +83,23 @@ class SystemConfig {
 	}
 
 	/**
+	 * Looks up a system wide defined value and filters out sensitive data
+	 *
+	 * @param string $key the key of the value, under which it was saved
+	 * @param mixed $default the default value to be returned if the value isn't set
+	 * @return mixed the value or $default
+	 */
+	public function getFilteredValue($key, $default = '') {
+		$value = $this->getValue($key, $default);
+
+		if (isset($this->sensitiveValues[$key])) {
+			$value = $this->removeSensitiveValue($this->sensitiveValues[$key], $value);
+		}
+
+		return $value;
+	}
+
+	/**
 	 * Delete a system wide defined value
 	 *
 	 * @param string $key the key of the value, under which it was saved
@@ -75,4 +107,25 @@ class SystemConfig {
 	public function deleteValue($key) {
 		\OC_Config::deleteKey($key);
 	}
+
+	/**
+	 * @param bool|array $keysToRemove
+	 * @param mixed $value
+	 * @return mixed
+	 */
+	protected function removeSensitiveValue($keysToRemove, $value) {
+		if ($keysToRemove === true) {
+			return self::SENSITIVE_VALUE;
+		}
+
+		if (is_array($value)) {
+			foreach ($keysToRemove as $keyToRemove => $valueToRemove) {
+				if (isset($value[$keyToRemove])) {
+					$value[$keyToRemove] = $this->removeSensitiveValue($valueToRemove, $value[$keyToRemove]);
+				}
+			}
+		}
+
+		return $value;
+	}
 }
diff --git a/lib/public/iconfig.php b/lib/public/iconfig.php
index ff0b6c6a5b0..933eef97ae1 100644
--- a/lib/public/iconfig.php
+++ b/lib/public/iconfig.php
@@ -41,6 +41,11 @@ namespace OCP;
  */
 interface IConfig {
 	/**
+	 * @since 8.2.0
+	 */
+	const SENSITIVE_VALUE = '***REMOVED SENSITIVE VALUE***';
+
+	/**
 	 * Sets and deletes system wide values
 	 *
 	 * @param array $configs Associative array with `key => value` pairs
@@ -69,6 +74,16 @@ interface IConfig {
 	public function getSystemValue($key, $default = '');
 
 	/**
+	 * Looks up a system wide defined value and filters out sensitive data
+	 *
+	 * @param string $key the key of the value, under which it was saved
+	 * @param mixed $default the default value to be returned if the value isn't set
+	 * @return mixed the value or $default
+	 * @since 8.2.0
+	 */
+	public function getFilteredSystemValue($key, $default = '');
+
+	/**
 	 * Delete a system wide defined value
 	 *
 	 * @param string $key the key of the value, under which it was saved
diff --git a/tests/core/command/config/listconfigstest.php b/tests/core/command/config/listconfigstest.php
index 7492701cce3..bde6a1b0db3 100644
--- a/tests/core/command/config/listconfigstest.php
+++ b/tests/core/command/config/listconfigstest.php
@@ -23,6 +23,7 @@ namespace Tests\Core\Command\Config;
 
 
 use OC\Core\Command\Config\ListConfigs;
+use OCP\IConfig;
 use Test\TestCase;
 
 class ListConfigsTest extends TestCase {
@@ -66,7 +67,7 @@ class ListConfigsTest extends TestCase {
 					'overwrite.cli.url',
 				],
 				[
-					['secret', 'N;', 'my secret'],
+					['secret', 'N;', IConfig::SENSITIVE_VALUE],
 					['overwrite.cli.url', 'N;', 'http://localhost'],
 				],
 				// app config
@@ -81,7 +82,7 @@ class ListConfigsTest extends TestCase {
 				false,
 				json_encode([
 					'system' => [
-						'secret' => ListConfigs::SENSITIVE_VALUE,
+						'secret' => IConfig::SENSITIVE_VALUE,
 						'overwrite.cli.url' => 'http://localhost',
 					],
 					'apps' => [
@@ -139,12 +140,12 @@ class ListConfigsTest extends TestCase {
 					'overwrite.cli.url',
 				],
 				[
-					['secret', 'N;', 'my secret'],
+					['secret', 'N;', IConfig::SENSITIVE_VALUE],
 					['objectstore', 'N;', [
 						'class' => 'OC\\Files\\ObjectStore\\Swift',
 						'arguments' => [
 							'username' => 'facebook100000123456789',
-							'password' => 'Secr3tPaSSWoRdt7',
+							'password' => IConfig::SENSITIVE_VALUE,
 						],
 					]],
 					['overwrite.cli.url', 'N;', 'http://localhost'],
@@ -161,12 +162,12 @@ class ListConfigsTest extends TestCase {
 				false,
 				json_encode([
 					'system' => [
-						'secret' => ListConfigs::SENSITIVE_VALUE,
+						'secret' => IConfig::SENSITIVE_VALUE,
 						'objectstore' => [
 							'class' => 'OC\\Files\\ObjectStore\\Swift',
 							'arguments' => [
 								'username' => 'facebook100000123456789',
-								'password' => ListConfigs::SENSITIVE_VALUE,
+								'password' => IConfig::SENSITIVE_VALUE,
 							],
 						],
 						'overwrite.cli.url' => 'http://localhost',
@@ -276,9 +277,15 @@ class ListConfigsTest extends TestCase {
 		$this->systemConfig->expects($this->any())
 			->method('getKeys')
 			->willReturn($systemConfigs);
-		$this->systemConfig->expects($this->any())
-			->method('getValue')
-			->willReturnMap($systemConfigMap);
+		if ($private) {
+			$this->systemConfig->expects($this->any())
+				->method('getValue')
+				->willReturnMap($systemConfigMap);
+		} else {
+			$this->systemConfig->expects($this->any())
+				->method('getFilteredValue')
+				->willReturnMap($systemConfigMap);
+		}
 
 		$this->appConfig->expects($this->any())
 			->method('getApps')