From 6b8ad965dcbf8d8bb347987aa0caea6e4f577dbb Mon Sep 17 00:00:00 2001 From: Petr Skoda Date: Sun, 6 Jun 2010 14:06:30 +0000 Subject: [PATCH] MDL-16919 we have to really use the username cleaning only when manually adding new accounts, any sync with external system needs the exact match without any cleaning! --- auth/cas/auth.php | 3 +-- auth/db/auth.php | 3 +-- auth/email/auth.php | 3 +-- auth/ldap/auth.php | 2 -- auth/mnet/auth.php | 1 - enrol/imsenterprise/enrol.php | 1 - enrol/mnet/enrol.php | 1 - lib/moodlelib.php | 40 ++++++----------------------------- login/index.php | 3 +-- login/signup_form.php | 7 +++--- user/editadvanced.php | 11 +++++----- user/editadvanced_form.php | 7 +++--- 12 files changed, 21 insertions(+), 61 deletions(-) diff --git a/auth/cas/auth.php b/auth/cas/auth.php index 06aa7bb274e..f2a498fedfb 100644 --- a/auth/cas/auth.php +++ b/auth/cas/auth.php @@ -98,7 +98,7 @@ class auth_plugin_cas extends auth_plugin_base { $username = optional_param("username", '', PARAM_RAW); if (!empty($username)) { - if (isset($SESSION->wantsurl) && (strstr($SESSION->wantsurl, 'ticket') || + if (isset($SESSION->wantsurl) && (strstr($SESSION->wantsurl, 'ticket') || strstr($SESSION->wantsurl, 'NOCAS'))) { unset($SESSION->wantsurl); } @@ -871,7 +871,6 @@ if ( !is_object($PHPCAS_CLIENT) ) { $user->lang = $CFG->lang; } - //TODO - username required to use PARAM_USERNAME before inserting into user table (MDL-16919) if ($id = $DB->insert_record('user', $user)) { echo "\t"; print_string('auth_dbinsertuser', 'auth_db', array('name'=>$user->username, 'id'=>$id)); echo "\n"; $userobj = $this->update_user_record($user->username); diff --git a/auth/db/auth.php b/auth/db/auth.php index dd1c60157cd..abc23f87e64 100644 --- a/auth/db/auth.php +++ b/auth/db/auth.php @@ -364,8 +364,7 @@ class auth_plugin_db extends auth_plugin_base { $user->id = $old_user->id; $DB->set_field('user', 'deleted', 0, array('username'=>$user->username)); echo "\t"; print_string('auth_dbreviveduser', 'auth_db', array('name'=>$user->username, 'id'=>$user->id)); echo "\n"; - - //TODO - username required to use PARAM_USERNAME before inserting into user table (MDL-16919) + } elseif ($id = $DB->insert_record ('user',$user)) { // it is truly a new user echo "\t"; print_string('auth_dbinsertuser','auth_db',array('name'=>$user->username, 'id'=>$id)); echo "\n"; // if relevant, tag for password generation diff --git a/auth/email/auth.php b/auth/email/auth.php index 938476f3abd..e4ab79333ce 100644 --- a/auth/email/auth.php +++ b/auth/email/auth.php @@ -73,13 +73,12 @@ class auth_plugin_email extends auth_plugin_base { * @param object $user new user object * @param boolean $notify print notice with link and terminate */ - function user_signup($user, $notify=true) { + function user_signup($user, $notify=true) { global $CFG, $DB; require_once($CFG->dirroot.'/user/profile/lib.php'); $user->password = hash_internal_user_password($user->password); - //TODO - username required to use PARAM_USERNAME before inserting into user table (MDL-16919) if (! ($user->id = $DB->insert_record('user', $user)) ) { print_error('auth_emailnoinsert','auth_email'); } diff --git a/auth/ldap/auth.php b/auth/ldap/auth.php index a172c466cba..0bac19c5fb0 100644 --- a/auth/ldap/auth.php +++ b/auth/ldap/auth.php @@ -419,7 +419,6 @@ class auth_plugin_ldap extends auth_plugin_base { print_error('auth_ldap_create_error', 'auth_ldap'); } - //TODO - username required to use PARAM_USERNAME before inserting into user table (MDL-16919) if (! ($user->id = $DB->insert_record('user', $user)) ) { print_error('auth_emailnoinsert', 'auth_email'); } @@ -796,7 +795,6 @@ class auth_plugin_ldap extends auth_plugin_base { $user->lang = $CFG->lang; } - //TODO - username required to use PARAM_USERNAME before inserting into user table (MDL-16919) if ($id = $DB->insert_record('user', $user)) { echo "\t"; print_string('auth_dbinsertuser', 'auth_db', array('name'=>$user->username, 'id'=>$id)); echo "\n"; $userobj = $this->update_user_record($user->username); diff --git a/auth/mnet/auth.php b/auth/mnet/auth.php index e80e4563816..55af2c550c6 100644 --- a/auth/mnet/auth.php +++ b/auth/mnet/auth.php @@ -270,7 +270,6 @@ class auth_plugin_mnet extends auth_plugin_base { $remoteuser->mnethostid = $remotehost->id; $remoteuser->firstaccess = time(); // First time user in this server, grab it here - //TODO - username required to use PARAM_USERNAME before inserting into user table (MDL-16919) $remoteuser->id = $DB->insert_record('user', $remoteuser); $firsttime = true; $localuser = $remoteuser; diff --git a/enrol/imsenterprise/enrol.php b/enrol/imsenterprise/enrol.php index b3491b1de07..479be871a2a 100644 --- a/enrol/imsenterprise/enrol.php +++ b/enrol/imsenterprise/enrol.php @@ -646,7 +646,6 @@ function process_person_tag($tagcontents){ $person->confirmed = 1; $person->timemodified = time(); $person->mnethostid = $CFG->mnet_localhost_id; - //TODO - username required to use PARAM_USERNAME before inserting into user table (MDL-16919) if($id = $DB->insert_record('user', $person)){ /* Photo processing is deactivated until we hear from Moodle dev forum about modification to gdlib. diff --git a/enrol/mnet/enrol.php b/enrol/mnet/enrol.php index e7cb0a9b912..388c875df18 100644 --- a/enrol/mnet/enrol.php +++ b/enrol/mnet/enrol.php @@ -306,7 +306,6 @@ class enrolment_plugin_mnet { */ $userrecord->mnethostid = $remoteclient->id; - //TODO - username required to use PARAM_USERNAME before inserting into user table (MDL-16919) if ($userrecord->id = $DB->insert_record('user', $userrecord)) { $userrecord = $DB->get_record('user', array('id'=>$userrecord->id)); } else { diff --git a/lib/moodlelib.php b/lib/moodlelib.php index f2f5e4ec30f..2f9e0a1e2a8 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -225,7 +225,7 @@ define('PARAM_THEME', 'theme'); define('PARAM_URL', 'url'); /** - * PARAM_USERNAME - Clean username to only contains specified characters. + * PARAM_USERNAME - Clean username to only contains allowed characters. This is to be used ONLY when manually creating user accounts, do NOT use when syncing with external systems!! */ define('PARAM_USERNAME', 'username'); @@ -505,34 +505,6 @@ function validate_param($param, $type, $allownull=NULL_NOT_ALLOWED, $debuginfo=' * $selectedgrade_item = clean_param($selectedgrade_item, PARAM_CLEAN); * * - * @global object - * @uses PARAM_RAW - * @uses PARAM_CLEAN - * @uses PARAM_CLEANHTML - * @uses PARAM_INT - * @uses PARAM_FLOAT - * @uses PARAM_NUMBER - * @uses PARAM_ALPHA - * @uses PARAM_ALPHAEXT - * @uses PARAM_ALPHANUM - * @uses PARAM_ALPHANUMEXT - * @uses PARAM_SEQUENCE - * @uses PARAM_BOOL - * @uses PARAM_NOTAGS - * @uses PARAM_TEXT - * @uses PARAM_SAFEDIR - * @uses PARAM_SAFEPATH - * @uses PARAM_FILE - * @uses PARAM_PATH - * @uses PARAM_HOST - * @uses PARAM_URL - * @uses PARAM_LOCALURL - * @uses PARAM_PEM - * @uses PARAM_BASE64 - * @uses PARAM_TAG - * @uses PARAM_SEQUENCE - * @uses PARAM_USERNAME - * @uses PARAM_STRINGID * @param mixed $param the variable we are cleaning * @param int $type expected format of param after cleaning. * @return mixed @@ -1056,7 +1028,7 @@ function get_users_from_config($value, $capability, $includeadmins = true) { // we have to make sure that users still have the necessary capability, // it should be faster to fetch them all first and then test if they are present - // instead of validating them one-by-one + // instead of validating them one-by-one $users = get_users_by_capability(get_context_instance(CONTEXT_SYSTEM), $capability); if ($includeadmins) { $admins = get_admins(); @@ -2606,8 +2578,8 @@ function delete_user_key($script,$userid) { function get_user_key($script, $userid, $instance=null, $iprestriction=null, $validuntil=null) { global $DB; - if ($key = $DB->get_record('user_private_key', array('script'=>$script, 'userid'=>$userid, - 'instance'=>$instance, 'iprestriction'=>$iprestriction, + if ($key = $DB->get_record('user_private_key', array('script'=>$script, 'userid'=>$userid, + 'instance'=>$instance, 'iprestriction'=>$iprestriction, 'validuntil'=>$validuntil))) { return $key->value; } else { @@ -3515,7 +3487,7 @@ function delete_user($user) { require_once($CFG->libdir.'/grouplib.php'); require_once($CFG->libdir.'/gradelib.php'); require_once($CFG->dirroot.'/message/lib.php'); - + // delete all grades - backup is kept in grade_grades_history table if ($grades = grade_grade::fetch_all(array('userid'=>$user->id))) { foreach ($grades as $grade) { @@ -4181,7 +4153,7 @@ function remove_course_contents($courseid, $showfeedback=true) { //trigger events events_trigger('course_content_removed', $course); - + return true; } diff --git a/login/index.php b/login/index.php index 243a85c7419..fb2dd389e69 100644 --- a/login/index.php +++ b/login/index.php @@ -105,8 +105,7 @@ if (empty($CFG->usesid) and $testcookies and (get_moodle_cookie() == '')) { / $frm->username = trim(moodle_strtolower($frm->username)); if (is_enabled_auth('none') ) { - $string = clean_param($frm->username, PARAM_USERNAME); - if (strcmp($frm->username, $string)) { + if ($frm->username !== clean_param($frm->username, PARAM_USERNAME)) { $errormsg = get_string('username').': '.get_string("invalidusername"); $errorcode = 2; $user = null; diff --git a/login/signup_form.php b/login/signup_form.php index ee89ec481a8..2dbd94dfd3f 100644 --- a/login/signup_form.php +++ b/login/signup_form.php @@ -106,12 +106,11 @@ class login_signup_form extends moodleform { //check allowed characters if ($data['username'] !== moodle_strtolower($data['username'])) { $errors['username'] = get_string('usernamelowercase'); - } else { - $string = clean_param($data['username'], PARAM_USERNAME); - if (strcmp($data['username'], $string)) { + } else { + if ($data['username'] !== clean_param($data['username'], PARAM_USERNAME)) { $errors['username'] = get_string('invalidusername'); } - + } } diff --git a/user/editadvanced.php b/user/editadvanced.php index d8de0669e4a..ddaf94c5819 100644 --- a/user/editadvanced.php +++ b/user/editadvanced.php @@ -62,7 +62,7 @@ if ($id == -1) { // creating new user $user = new object(); $user->id = -1; - $user->auth = 'manual'; + $user->auth = 'manual'; $user->confirmed = 1; $user->deleted = 0; require_capability('moodle/user:create', $systemcontext); @@ -131,9 +131,8 @@ if ($usernew = $userform->get_data()) { } else { $authplugin = get_auth_plugin($usernew->auth); } - - $usernew->username = clean_param($usernew->username, PARAM_USERNAME); - $usernew->timemodified = time(); + + $usernew->timemodified = time(); if ($usernew->id == -1) { //TODO check out if it makes sense to create account with this auth plugin and what to do with the password @@ -142,7 +141,7 @@ if ($usernew = $userform->get_data()) { $usernew->mnethostid = $CFG->mnet_localhost_id; // always local user $usernew->confirmed = 1; $usernew->timecreated = time(); - $usernew->password = hash_internal_user_password($usernew->newpassword); + $usernew->password = hash_internal_user_password($usernew->newpassword); $usernew->id = $DB->insert_record('user', $usernew); $usercreated = true; @@ -220,7 +219,7 @@ if ($usernew = $userform->get_data()) { redirect("$CFG->wwwroot/user/view.php?id=$USER->id&course=$course->id"); } } else { - session_gc(); // remove stale sessions + session_gc(); // remove stale sessions redirect("$CFG->wwwroot/$CFG->admin/user.php"); } //never reached diff --git a/user/editadvanced_form.php b/user/editadvanced_form.php index 4e9ac141af8..371da240be2 100644 --- a/user/editadvanced_form.php +++ b/user/editadvanced_form.php @@ -114,7 +114,7 @@ class user_editadvanced_form extends moodleform { profile_definition_after_data($mform, $userid); } - function validation($usernew, $files) { + function validation($usernew, $files) { global $CFG, $DB; $usernew = (object)$usernew; @@ -141,9 +141,8 @@ class user_editadvanced_form extends moodleform { //check allowed characters if ($usernew->username !== moodle_strtolower($usernew->username)) { $err['username'] = get_string('usernamelowercase'); - } else { - $string = clean_param($usernew->username, PARAM_USERNAME); - if ($usernew->username !== $string) { + } else { + if ($usernew->username !== clean_param($usernew->username, PARAM_USERNAME)) { $err['username'] = get_string('invalidusername'); } }