From faba02564a24187e69ebe274078793d66fd1a2a2 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 25 Sep 2015 11:08:33 +0200 Subject: [PATCH] Move the filtering of sensitive data to the config class --- core/command/config/listconfigs.php | 43 ++------------- lib/private/allconfig.php | 11 ++++ lib/private/systemconfig.php | 53 +++++++++++++++++++ lib/public/iconfig.php | 15 ++++++ tests/core/command/config/listconfigstest.php | 25 +++++---- 5 files changed, 99 insertions(+), 48 deletions(-) diff --git a/core/command/config/listconfigs.php b/core/command/config/listconfigs.php index 5796362f2f..37aeb53c6f 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 63cc92601b..7c2037e804 100644 --- a/lib/private/allconfig.php +++ b/lib/private/allconfig.php @@ -118,6 +118,17 @@ class AllConfig implements \OCP\IConfig { return $this->systemConfig->getValue($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 + */ + public function getFilteredSystemValue($key, $default = '') { + return $this->systemConfig->getFilteredValue($key, $default); + } + /** * Delete a system wide defined value * diff --git a/lib/private/systemconfig.php b/lib/private/systemconfig.php index 13b0959768..3b7930f284 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 @@ -67,6 +82,23 @@ class SystemConfig { return \OC_Config::getValue($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 + */ + 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 * @@ -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 ff0b6c6a5b..933eef97ae 100644 --- a/lib/public/iconfig.php +++ b/lib/public/iconfig.php @@ -40,6 +40,11 @@ namespace OCP; * @since 6.0.0 */ interface IConfig { + /** + * @since 8.2.0 + */ + const SENSITIVE_VALUE = '***REMOVED SENSITIVE VALUE***'; + /** * Sets and deletes system wide values * @@ -68,6 +73,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 * diff --git a/tests/core/command/config/listconfigstest.php b/tests/core/command/config/listconfigstest.php index 7492701cce..bde6a1b0db 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')