From 822fe4fc155af73b407c5d23b905be1470995d3b Mon Sep 17 00:00:00 2001 From: Mark Nelson Date: Wed, 17 Jan 2018 13:32:27 +0800 Subject: [PATCH] MDL-60814 auth_ldap: prevent setting names breaking site upgrades --- auth/ldap/lang/en/auth_ldap.php | 1 + auth/ldap/settings.php | 20 +++++++++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/auth/ldap/lang/en/auth_ldap.php b/auth/ldap/lang/en/auth_ldap.php index fb61dd14cfa..5629542f221 100644 --- a/auth/ldap/lang/en/auth_ldap.php +++ b/auth/ldap/lang/en/auth_ldap.php @@ -113,6 +113,7 @@ $string['auth_ntlmsso_subnet'] = 'If set, it will only attempt SSO with clients $string['auth_ntlmsso_subnet_key'] = 'Subnet'; $string['auth_ntlmsso_type_key'] = 'Authentication type'; $string['auth_ntlmsso_type'] = 'The authentication method configured in the web server to authenticate the users (if in doubt, choose NTLM)'; +$string['cannotmaprole'] = 'The role "{$a->rolename}" can\'t be mapped because its short name "{$a->shortname}" is too long or contains hyphens. To allow it to be mapped, you need to reduce the short name to {$a->charlimit} characters or remove the hyphens. Edit the role here'; $string['connectingldap'] = "Connecting to LDAP server...\n"; $string['connectingldapsuccess'] = "Connecting to your LDAP server was successful"; $string['creatingtemptable'] = "Creating temporary table {\$a}\n"; diff --git a/auth/ldap/settings.php b/auth/ldap/settings.php index 43fb8eb2856..f9bc8d9b8f3 100644 --- a/auth/ldap/settings.php +++ b/auth/ldap/settings.php @@ -245,9 +245,27 @@ if ($ADMIN->fulltree) { // Create system role mapping field for each assignable system role. $roles = get_ldap_assignable_role_names(); foreach ($roles as $role) { - $settings->add(new admin_setting_configtext('auth_ldap/' . $role['settingname'], + // Before we can add this setting we need to check a few things. + // A) It does not exceed 100 characters otherwise it will break the DB as the 'name' field + // in the 'config_plugins' table is a varchar(100). + // B) The setting name does not contain hyphens. If it does then it will fail the check + // in parse_setting_name() and everything will explode. Role short names are validated + // against PARAM_ALPHANUMEXT which is similar to the regex used in parse_setting_name() + // except it also allows hyphens. + // Instead of shortening the name and removing/replacing the hyphens we are showing a warning. + // If we were to manipulate the setting name by removing the hyphens we may get conflicts, eg + // 'thisisashortname' and 'this-is-a-short-name'. The same applies for shortening the setting name. + if (core_text::strlen($role['settingname']) > 100 || !preg_match('/^[a-zA-Z0-9_]+$/', $role['settingname'])) { + $url = new moodle_url('/admin/roles/define.php', array('action' => 'edit', 'roleid' => $role['id'])); + $a = (object)['rolename' => $role['localname'], 'shortname' => $role['shortname'], 'charlimit' => 93, + 'link' => $url->out()]; + $settings->add(new admin_setting_heading('auth_ldap/role_not_mapped_' . sha1($role['settingname']), '', + get_string('cannotmaprole', 'auth_ldap', $a))); + } else { + $settings->add(new admin_setting_configtext('auth_ldap/' . $role['settingname'], get_string('auth_ldap_rolecontext', 'auth_ldap', $role), get_string('auth_ldap_rolecontext_help', 'auth_ldap', $role), '', PARAM_RAW_TRIMMED)); + } } // User Account Sync.