diff --git a/lib/adminlib.php b/lib/adminlib.php index a190faa9592..41c86ef0012 100644 --- a/lib/adminlib.php +++ b/lib/adminlib.php @@ -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 diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index 0a80bd0ad24..b0f8134cf85 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -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; } diff --git a/lib/tests/admintree_test.php b/lib/tests/admintree_test.php index 098cc556905..e8389deafa9 100644 --- a/lib/tests/admintree_test.php +++ b/lib/tests/admintree_test.php @@ -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; + } } diff --git a/version.php b/version.php index 1b30ffb1294..943c5d3e078 100644 --- a/version.php +++ b/version.php @@ -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.