MDL-36721 do not store passwords in config logs
This commit is contained in:
+27
-1
@@ -1660,11 +1660,21 @@ abstract class admin_setting {
|
||||
rebuild_course_cache(0, true);
|
||||
}
|
||||
|
||||
add_to_config_log($name, $oldvalue, $value, $this->plugin);
|
||||
$this->add_to_config_log($name, $oldvalue, $value);
|
||||
|
||||
return true; // BC only
|
||||
}
|
||||
|
||||
/**
|
||||
* Log config changes if necessary.
|
||||
* @param string $name
|
||||
* @param string $oldvalue
|
||||
* @param string $value
|
||||
*/
|
||||
protected function add_to_config_log($name, $oldvalue, $value) {
|
||||
add_to_config_log($name, $oldvalue, $value, $this->plugin);
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns current value of this setting
|
||||
* @return mixed array or string depending on instance, NULL means not set yet
|
||||
@@ -2161,6 +2171,22 @@ class admin_setting_configpasswordunmask extends admin_setting_configtext {
|
||||
parent::__construct($name, $visiblename, $description, $defaultsetting, PARAM_RAW, 30);
|
||||
}
|
||||
|
||||
/**
|
||||
* Log config changes if necessary.
|
||||
* @param string $name
|
||||
* @param string $oldvalue
|
||||
* @param string $value
|
||||
*/
|
||||
protected function add_to_config_log($name, $oldvalue, $value) {
|
||||
if ($value !== '') {
|
||||
$value = '********';
|
||||
}
|
||||
if ($oldvalue !== '' and $oldvalue !== null) {
|
||||
$oldvalue = '********';
|
||||
}
|
||||
parent::add_to_config_log($name, $oldvalue, $value);
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns XHTML for the field
|
||||
* Writes Javascript into the HTML below right before the last div
|
||||
|
||||
@@ -2862,5 +2862,44 @@ function xmldb_main_upgrade($oldversion) {
|
||||
upgrade_main_savepoint(true, 2013111800.01);
|
||||
}
|
||||
|
||||
if ($oldversion < 2013122400.01) {
|
||||
// Purge stored passwords from config_log table, ideally this should be in each plugin
|
||||
// but that would complicate backporting...
|
||||
$items = array(
|
||||
'core/cronremotepassword', 'core/proxypassword', 'core/smtppass', 'core/jabberpassword',
|
||||
'enrol_database/dbpass', 'enrol_ldap/bind_pw', 'url/secretphrase');
|
||||
foreach ($items as $item) {
|
||||
list($plugin, $name) = explode('/', $item);
|
||||
if ($plugin === 'core') {
|
||||
$sql = "UPDATE {config_log}
|
||||
SET value = :value
|
||||
WHERE name = :name AND plugin IS NULL AND value <> ''";
|
||||
$params = array('value'=>'********', 'name'=>$name);
|
||||
$DB->execute($sql, $params);
|
||||
|
||||
$sql = "UPDATE {config_log}
|
||||
SET oldvalue = :value
|
||||
WHERE name = :name AND plugin IS NULL AND oldvalue <> ''";
|
||||
$params = array('value'=>'********', 'name'=>$name);
|
||||
$DB->execute($sql, $params);
|
||||
|
||||
} else {
|
||||
$sql = "UPDATE {config_log}
|
||||
SET value = :value
|
||||
WHERE name = :name AND plugin = :plugin AND value <> ''";
|
||||
$params = array('value'=>'********', 'name'=>$name, 'plugin'=>$plugin);
|
||||
$DB->execute($sql, $params);
|
||||
|
||||
$sql = "UPDATE {config_log}
|
||||
SET oldvalue = :value
|
||||
WHERE name = :name AND plugin = :plugin AND oldvalue <> ''";
|
||||
$params = array('value'=>'********', 'name'=>$name, 'plugin'=>$plugin);
|
||||
$DB->execute($sql, $params);
|
||||
}
|
||||
}
|
||||
// Main savepoint reached.
|
||||
upgrade_main_savepoint(true, 2013122400.01);
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -110,4 +110,104 @@ class core_admintree_testcase extends advanced_testcase {
|
||||
$tree = new admin_root(true);
|
||||
$tree->add('root', new admin_category('bar', 'Bar'), '');
|
||||
}
|
||||
|
||||
/**
|
||||
* Saving of values.
|
||||
*/
|
||||
public function test_config_logging() {
|
||||
global $DB;
|
||||
$this->resetAfterTest();
|
||||
|
||||
$DB->delete_records('config_log', array());
|
||||
|
||||
$adminroot = new admin_root(true);
|
||||
$adminroot->add('root', $one = new admin_category('one', 'One'));
|
||||
$page = new admin_settingpage('page', 'Page');
|
||||
$page->add(new admin_setting_configtext('text1', 'Text 1', '', ''));
|
||||
$page->add(new admin_setting_configpasswordunmask('pass1', 'Password 1', '', ''));
|
||||
$adminroot->add('one', $page);
|
||||
|
||||
$this->assertEmpty($DB->get_records('config_log'));
|
||||
$data = array('s__text1'=>'sometext', 's__pass1'=>'');
|
||||
$count = $this->save_config_data($adminroot, $data);
|
||||
|
||||
$this->assertEquals(2, $count);
|
||||
$records = $DB->get_records('config_log', array(), 'id asc');
|
||||
$this->assertCount(2, $records);
|
||||
reset($records);
|
||||
$record = array_shift($records);
|
||||
$this->assertNull($record->plugin);
|
||||
$this->assertSame('text1', $record->name);
|
||||
$this->assertNull($record->oldvalue);
|
||||
$this->assertSame('sometext', $record->value);
|
||||
$record = array_shift($records);
|
||||
$this->assertNull($record->plugin);
|
||||
$this->assertSame('pass1', $record->name);
|
||||
$this->assertNull($record->oldvalue);
|
||||
$this->assertSame('', $record->value);
|
||||
|
||||
$DB->delete_records('config_log', array());
|
||||
$data = array('s__text1'=>'other', 's__pass1'=>'nice password');
|
||||
$count = $this->save_config_data($adminroot, $data);
|
||||
|
||||
$this->assertEquals(2, $count);
|
||||
$records = $DB->get_records('config_log', array(), 'id asc');
|
||||
$this->assertCount(2, $records);
|
||||
reset($records);
|
||||
$record = array_shift($records);
|
||||
$this->assertNull($record->plugin);
|
||||
$this->assertSame('text1', $record->name);
|
||||
$this->assertSame('sometext', $record->oldvalue);
|
||||
$this->assertSame('other', $record->value);
|
||||
$record = array_shift($records);
|
||||
$this->assertNull($record->plugin);
|
||||
$this->assertSame('pass1', $record->name);
|
||||
$this->assertSame('', $record->oldvalue);
|
||||
$this->assertSame('********', $record->value);
|
||||
|
||||
$DB->delete_records('config_log', array());
|
||||
$data = array('s__text1'=>'', 's__pass1'=>'');
|
||||
$count = $this->save_config_data($adminroot, $data);
|
||||
|
||||
$this->assertEquals(2, $count);
|
||||
$records = $DB->get_records('config_log', array(), 'id asc');
|
||||
$this->assertCount(2, $records);
|
||||
reset($records);
|
||||
$record = array_shift($records);
|
||||
$this->assertNull($record->plugin);
|
||||
$this->assertSame('text1', $record->name);
|
||||
$this->assertSame('other', $record->oldvalue);
|
||||
$this->assertSame('', $record->value);
|
||||
$record = array_shift($records);
|
||||
$this->assertNull($record->plugin);
|
||||
$this->assertSame('pass1', $record->name);
|
||||
$this->assertSame('********', $record->oldvalue);
|
||||
$this->assertSame('', $record->value);
|
||||
}
|
||||
|
||||
protected function save_config_data(admin_root $adminroot, array $data) {
|
||||
$adminroot->errors = array();
|
||||
|
||||
$settings = admin_find_write_settings($adminroot, $data);
|
||||
|
||||
$count = 0;
|
||||
foreach ($settings as $fullname=>$setting) {
|
||||
/** @var $setting admin_setting */
|
||||
$original = $setting->get_setting();
|
||||
$error = $setting->write_setting($data[$fullname]);
|
||||
if ($error !== '') {
|
||||
$adminroot->errors[$fullname] = new stdClass();
|
||||
$adminroot->errors[$fullname]->data = $data[$fullname];
|
||||
$adminroot->errors[$fullname]->id = $setting->get_id();
|
||||
$adminroot->errors[$fullname]->error = $error;
|
||||
} else {
|
||||
$setting->write_setting_flags($data);
|
||||
}
|
||||
if ($setting->post_write_settings($original)) {
|
||||
$count++;
|
||||
}
|
||||
}
|
||||
|
||||
return $count;
|
||||
}
|
||||
}
|
||||
|
||||
+1
-1
@@ -29,7 +29,7 @@
|
||||
|
||||
defined('MOODLE_INTERNAL') || die();
|
||||
|
||||
$version = 2013122400.00; // YYYYMMDD = weekly release date of this DEV branch.
|
||||
$version = 2013122400.01; // YYYYMMDD = weekly release date of this DEV branch.
|
||||
// RR = release increments - 00 in DEV branches.
|
||||
// .XX = incremental changes.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user