Move the filtering of sensitive data to the config class

This commit is contained in:
Joas Schilling 2015-09-25 11:08:33 +02:00
parent bf73665a35
commit faba02564a
5 changed files with 99 additions and 48 deletions

View File

@ -32,20 +32,6 @@ use Symfony\Component\Console\Output\OutputInterface;
class ListConfigs extends Base { class ListConfigs extends Base {
protected $defaultOutputFormat = self::OUTPUT_FORMAT_JSON_PRETTY; 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 */ /** * @var SystemConfig */
protected $systemConfig; protected $systemConfig;
@ -127,10 +113,10 @@ class ListConfigs extends Base {
$configs = []; $configs = [];
foreach ($keys as $key) { foreach ($keys as $key) {
$value = $this->systemConfig->getValue($key, serialize(null)); if ($noSensitiveValues) {
$value = $this->systemConfig->getFilteredValue($key, serialize(null));
if ($noSensitiveValues && isset($this->sensitiveValues[$key])) { } else {
$value = $this->removeSensitiveValue($this->sensitiveValues[$key], $value); $value = $this->systemConfig->getValue($key, serialize(null));
} }
if ($value !== 'N;') { if ($value !== 'N;') {
@ -140,25 +126,4 @@ class ListConfigs extends Base {
return $configs; 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;
}
} }

View File

@ -118,6 +118,17 @@ class AllConfig implements \OCP\IConfig {
return $this->systemConfig->getValue($key, $default); 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 * Delete a system wide defined value
* *

View File

@ -28,6 +28,21 @@ namespace OC;
* fixes cyclic DI: AllConfig needs AppConfig needs Database needs AllConfig * fixes cyclic DI: AllConfig needs AppConfig needs Database needs AllConfig
*/ */
class SystemConfig { 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 * Lists all available config keys
* @return array an array of key names * @return array an array of key names
@ -67,6 +82,23 @@ class SystemConfig {
return \OC_Config::getValue($key, $default); 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 * Delete a system wide defined value
* *
@ -75,4 +107,25 @@ class SystemConfig {
public function deleteValue($key) { public function deleteValue($key) {
\OC_Config::deleteKey($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;
}
} }

View File

@ -40,6 +40,11 @@ namespace OCP;
* @since 6.0.0 * @since 6.0.0
*/ */
interface IConfig { interface IConfig {
/**
* @since 8.2.0
*/
const SENSITIVE_VALUE = '***REMOVED SENSITIVE VALUE***';
/** /**
* Sets and deletes system wide values * Sets and deletes system wide values
* *
@ -68,6 +73,16 @@ interface IConfig {
*/ */
public function getSystemValue($key, $default = ''); 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 * Delete a system wide defined value
* *

View File

@ -23,6 +23,7 @@ namespace Tests\Core\Command\Config;
use OC\Core\Command\Config\ListConfigs; use OC\Core\Command\Config\ListConfigs;
use OCP\IConfig;
use Test\TestCase; use Test\TestCase;
class ListConfigsTest extends TestCase { class ListConfigsTest extends TestCase {
@ -66,7 +67,7 @@ class ListConfigsTest extends TestCase {
'overwrite.cli.url', 'overwrite.cli.url',
], ],
[ [
['secret', 'N;', 'my secret'], ['secret', 'N;', IConfig::SENSITIVE_VALUE],
['overwrite.cli.url', 'N;', 'http://localhost'], ['overwrite.cli.url', 'N;', 'http://localhost'],
], ],
// app config // app config
@ -81,7 +82,7 @@ class ListConfigsTest extends TestCase {
false, false,
json_encode([ json_encode([
'system' => [ 'system' => [
'secret' => ListConfigs::SENSITIVE_VALUE, 'secret' => IConfig::SENSITIVE_VALUE,
'overwrite.cli.url' => 'http://localhost', 'overwrite.cli.url' => 'http://localhost',
], ],
'apps' => [ 'apps' => [
@ -139,12 +140,12 @@ class ListConfigsTest extends TestCase {
'overwrite.cli.url', 'overwrite.cli.url',
], ],
[ [
['secret', 'N;', 'my secret'], ['secret', 'N;', IConfig::SENSITIVE_VALUE],
['objectstore', 'N;', [ ['objectstore', 'N;', [
'class' => 'OC\\Files\\ObjectStore\\Swift', 'class' => 'OC\\Files\\ObjectStore\\Swift',
'arguments' => [ 'arguments' => [
'username' => 'facebook100000123456789', 'username' => 'facebook100000123456789',
'password' => 'Secr3tPaSSWoRdt7', 'password' => IConfig::SENSITIVE_VALUE,
], ],
]], ]],
['overwrite.cli.url', 'N;', 'http://localhost'], ['overwrite.cli.url', 'N;', 'http://localhost'],
@ -161,12 +162,12 @@ class ListConfigsTest extends TestCase {
false, false,
json_encode([ json_encode([
'system' => [ 'system' => [
'secret' => ListConfigs::SENSITIVE_VALUE, 'secret' => IConfig::SENSITIVE_VALUE,
'objectstore' => [ 'objectstore' => [
'class' => 'OC\\Files\\ObjectStore\\Swift', 'class' => 'OC\\Files\\ObjectStore\\Swift',
'arguments' => [ 'arguments' => [
'username' => 'facebook100000123456789', 'username' => 'facebook100000123456789',
'password' => ListConfigs::SENSITIVE_VALUE, 'password' => IConfig::SENSITIVE_VALUE,
], ],
], ],
'overwrite.cli.url' => 'http://localhost', 'overwrite.cli.url' => 'http://localhost',
@ -276,9 +277,15 @@ class ListConfigsTest extends TestCase {
$this->systemConfig->expects($this->any()) $this->systemConfig->expects($this->any())
->method('getKeys') ->method('getKeys')
->willReturn($systemConfigs); ->willReturn($systemConfigs);
$this->systemConfig->expects($this->any()) if ($private) {
->method('getValue') $this->systemConfig->expects($this->any())
->willReturnMap($systemConfigMap); ->method('getValue')
->willReturnMap($systemConfigMap);
} else {
$this->systemConfig->expects($this->any())
->method('getFilteredValue')
->willReturnMap($systemConfigMap);
}
$this->appConfig->expects($this->any()) $this->appConfig->expects($this->any())
->method('getApps') ->method('getApps')