From 07ed083e4e99e18db440341a56a2e8b6ef11df13 Mon Sep 17 00:00:00 2001 From: Rossiani Wijaya Date: Wed, 13 Jan 2010 06:23:54 +0000 Subject: [PATCH] MDL-16919 - Allow username to contain alphanumeric lowercase characters, underscore (_), hyphen (-), period (.) or at symbol (@) --- admin/uploaduser.php | 119 ++++++++++++++++++++++++------- admin/uploaduser_form.php | 7 ++ auth/cas/auth.php | 1 + auth/db/auth.php | 2 + auth/email/auth.php | 5 +- auth/ldap/auth.php | 2 + auth/mnet/auth.php | 2 + enrol/imsenterprise/enrol.php | 1 + enrol/mnet/enrol.php | 1 + lang/en_utf8/admin.php | 2 +- lang/en_utf8/moodle.php | 7 +- lib/moodlelib.php | 16 +++++ lib/simpletest/testmoodlelib.php | 30 ++++++++ login/index.php | 2 +- login/signup_form.php | 15 ++-- user/editadvanced.php | 14 ++-- user/editadvanced_form.php | 6 +- 17 files changed, 187 insertions(+), 45 deletions(-) diff --git a/admin/uploaduser.php b/admin/uploaduser.php index b8dce1997b..54073eb361 100755 --- a/admin/uploaduser.php +++ b/admin/uploaduser.php @@ -265,10 +265,8 @@ if ($formdata = $mform->is_cancelled()) { } // normalize username - $user->username = $textlib->strtolower($user->username); - if (empty($CFG->extendedusernamechars)) { - $user->username = preg_replace('/[^(-\.[:alnum:])]/i', '', $user->username); - } + $user->username = clean_param($user->username, PARAM_USERNAME); + if (empty($user->username)) { $upt->track('status', get_string('missingfield', 'error', 'username'), 'error'); $upt->track('username', $errorstr, 'error'); @@ -748,36 +746,106 @@ admin_externalpage_print_header(); echo $OUTPUT->heading_with_help(get_string('uploaduserspreview', 'admin'), 'uploadusers2'); -$ci = 0; $ri = 0; - -echo ''; -echo ''; -foreach ($columns as $col) { - echo ''; -} -echo ''; - $cir->init(); + +$filerows = array(); while ($fields = $cir->next()) { + $ci = 0; if ($ri > $previewrows) { - echo ''; + $arow .= ''; foreach ($fields as $field) { - echo '';; + $arow .= ''; } + $arow .= ''; break; } - $ci = 0; - echo ''; - foreach ($fields as $field) { - echo '';; + + $arow = ''; + $errormsg = array(); + $filerows[$ri] = array(); + $arow .= ''; + foreach ($fields as $key => $field) { + $errorclass = ''; + if ($ci == 0) { //username field + $newusername = clean_param($field, PARAM_USERNAME); + if (strcmp($field, $newusername) != 0){ + $errorclass = 'uuerror'; + $errormsg[] = get_string('invalidusernameupload'); + } else if ($DB->record_exists('user', array('username'=>$field)) || $DB->record_exists('user', array('username'=>$field)) ) { + $errorclass = 'uuerror'; + $errormsg[] = get_string('usernameexists'); + } + } else { + if ($ci == 4) { //email field + if ($DB->record_exists('user', array('email'=>$field))) { + $errorclass = 'uuerror'; + $errormsg[] = get_string('emailexists'); + + } else if (!validate_email($field)) { + $errorclass = 'uuerror'; + $errormsg[] = get_string('invalidemail'); + } + } + } + $arow .= ''; + + if ($key == (count($fields) - 1) && !empty($errormsg)) { + $errormsg = implode('
', $errormsg); + $arow .= ''; + $filerows[$ri]['error'] = $errormsg; + } } - echo ''; -} + $arow .= ''; + $filerows[$ri]['content'] = $arow; + $ri++; +} $cir->close(); +$validcontent = array(); +$invalidcontent = array(); +foreach ($filerows as $arow) { + if (array_key_exists('error', $arow)){ + $invalidcontent[] = $arow['content']; + $mform = new admin_uploaduser_form3(); + } else { + $validcontent[] = $arow['content']; + } +} + +$ri = 0; +$ci = 0; +echo '
'.s($col).'
......
'.s($field).'
'; + $arow .= '
'.s($field). '
'; + $arow .= '		' . $errormsg . '
'; +echo ''; +foreach ($columns as $col) { + echo ''; +} +if (!empty($invalidcontent)) { + echo ''; +} +echo ''; + +$countcontent = 0; +if (empty($invalidcontent)) { + $countcontent = count($validcontent); + echo implode('', $validcontent); +} else { + $countcontent = count($invalidcontent); + echo implode('', $invalidcontent); +} echo '
'.s($col).''.moodle_strtolower(get_string('error', 'moodle')).'
'; -echo '
'.get_string('uupreprocessedcount', 'admin', $readcount).'
'; + +if (!empty($invalidcontent)) { + echo '
'.get_string('uploadinvalidpreprocessedcount', 'moodle', $countcontent).'
'; + echo '
'.get_string('invalidusername', 'moodle').'
'; + echo '
'.get_string('uploadfilecontainerror', 'moodle').'
'; +} else { + echo '
'.get_string('uupreprocessedcount', 'admin', $countcontent).'
'; +} + +//echo '
'.get_string('uupreprocessedcount', 'admin', $readcount).'
'; $mform->display(); echo $OUTPUT->footer(); die; @@ -822,10 +890,13 @@ class uu_progress_tracker { } $ci = 0; $ri = 1; - echo ''; - foreach ($this->_row as $field) { + echo ''; + foreach ($this->_row as $key=>$field) { foreach ($field as $type=>$content) { if ($field[$type] !== '') { + if ($key == 'username' && $type == 'normal') { + $field[$type] = clean_param($field[$type], PARAM_USERNAME); + } $field[$type] = ''.$field[$type].''; } else { unset($field[$type]); diff --git a/admin/uploaduser_form.php b/admin/uploaduser_form.php index 19da95500a..2525927e08 100644 --- a/admin/uploaduser_form.php +++ b/admin/uploaduser_form.php @@ -353,3 +353,10 @@ class admin_uploaduser_form2 extends moodleform { } } +class admin_uploaduser_form3 extends moodleform { + function definition (){ + global $CFG, $USER; + $mform =& $this->_form; + $this->add_action_buttons(false, get_string('uploadnewfile')); + } +} diff --git a/auth/cas/auth.php b/auth/cas/auth.php index b30b32fa63..e1f147569a 100644 --- a/auth/cas/auth.php +++ b/auth/cas/auth.php @@ -863,6 +863,7 @@ 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($user->username, $id)); echo "\n"; $userobj = $this->update_user_record($user->username); diff --git a/auth/db/auth.php b/auth/db/auth.php index 441911cb1e..57c1ebf75f 100644 --- a/auth/db/auth.php +++ b/auth/db/auth.php @@ -364,6 +364,8 @@ 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_dbreviveuser', 'auth_db', array($user->username, $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($user->username, $id)); echo "\n"; // if relevant, tag for password generation diff --git a/auth/email/auth.php b/auth/email/auth.php index a1bb6e4ee8..f6e8204269 100644 --- a/auth/email/auth.php +++ b/auth/email/auth.php @@ -73,12 +73,13 @@ 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'); } @@ -94,7 +95,7 @@ class auth_plugin_email extends auth_plugin_base { } if ($notify) { - global $CFG; + global $CFG, $PAGE, $OUTPUT; $emailconfirm = get_string('emailconfirm'); $PAGE->navbar->add($emailconfirm); $PAGE->set_title($emailconfirm); diff --git a/auth/ldap/auth.php b/auth/ldap/auth.php index 2de694cb1e..33a17f4d53 100644 --- a/auth/ldap/auth.php +++ b/auth/ldap/auth.php @@ -419,6 +419,7 @@ 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'); } @@ -795,6 +796,7 @@ 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($user->username, $id)); echo "\n"; $userobj = $this->update_user_record($user->username); diff --git a/auth/mnet/auth.php b/auth/mnet/auth.php index 41a79adff5..a03a8db6b5 100644 --- a/auth/mnet/auth.php +++ b/auth/mnet/auth.php @@ -298,6 +298,8 @@ 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 96cfbf13a4..2f70acc50b 100644 --- a/enrol/imsenterprise/enrol.php +++ b/enrol/imsenterprise/enrol.php @@ -646,6 +646,7 @@ 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 6ce416390f..a541c5d824 100644 --- a/enrol/mnet/enrol.php +++ b/enrol/mnet/enrol.php @@ -310,6 +310,7 @@ class enrolment_plugin_mnet { $userrecord->lastname = $user['lastname']; $userrecord->mnethostid = $MNET_REMOTE_CLIENT->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/lang/en_utf8/admin.php b/lang/en_utf8/admin.php index c330391196..60bc3cc20d 100644 --- a/lang/en_utf8/admin.php +++ b/lang/en_utf8/admin.php @@ -165,7 +165,7 @@ $string['configenablewsdocumentation'] = 'Enable auto-generation of web services $string['configenrolmentplugins'] = 'Please choose the enrolment plugins you wish to use. Don\'t forget to configure the settings properly.

You have to indicate which plugins are enabled, and one plugin can be set as the default plugin for interactive enrolment. To disable interactive enrolment, set \"enrollable\" to \"No\" in required courses.'; $string['configerrorlevel'] = 'Choose the amount of PHP warnings that you want to be displayed. Normal is usually the best choice.'; $string['configexperimentalsplitrestore'] = 'If enabled, course backup files will be checked for XML errors and split into smaller parts for use in the restore process. This will result in improvements to restore robustness and execution times, particularly for medium to large course backups.'; -$string['configextendedusernamechars'] = 'Enable this setting to allow students to use any characters in their usernames (note this does not affect their actual names). The default is \"false\" which restricts usernames to be alphanumeric characters only'; +$string['configextendedusernamechars'] = 'Enable this setting to allow students to use any characters in their usernames (note this does not affect their actual names). The default is \"false\" which restricts usernames to be alphanumeric lowercase characters, underscore (_), hyphen (-), period (.) or at symbol (@).'; $string['configextramemorylimit'] = 'Some scripts like search, backup/restore or cron require more memory. Set higher values for large sites.'; $string['configextrauserselectorfields'] = 'Select which fields are searched and displayed, in addition to full names, when selecting users, for example when assigning roles or when adding users to a group. For security reasons, it is recommended that the username field is NOT selected.'; $string['configfilterall'] = 'Filter all strings, including headings, titles, navigation bar and so on. This is mostly useful when using the multilang filter, otherwise it will just create extra load on your site for little gain.'; diff --git a/lang/en_utf8/moodle.php b/lang/en_utf8/moodle.php index 53c99562b4..47bc55cc52 100644 --- a/lang/en_utf8/moodle.php +++ b/lang/en_utf8/moodle.php @@ -112,7 +112,9 @@ $string['allownot'] = 'Do not allow'; $string['allparticipants'] = 'All participants'; $string['allteachers'] = 'All teachers'; $string['alphabet'] = 'A,B,C,D,E,F,G,H,I,J,K,L,M,N,O,P,Q,R,S,T,U,V,W,X,Y,Z'; -$string['alphanumerical'] = 'Can only contain alphabetical letters or numbers'; +$string['alphanumerical'] = 'Can only contain alphanumeric characters, hyphen (-) or period (.)'; +$string['invalidusername'] = 'The username can only contain alphanumeric lowercase characters, underscore (_), hyphen (-), period (.) or at symbol (@)'; +$string['invalidusernameupload'] = "Invalid username"; $string['alreadyconfirmed'] = 'Registration has already been confirmed'; $string['always'] = 'Always'; $string['and'] = '$a->one and $a->two'; @@ -1683,6 +1685,9 @@ $string['uploadrenamedcollision'] = 'File was renamed from $a->oldname to $a->ne $string['uploadserverlimit'] = 'Uploaded file exceeded the maximum size limit set by the server'; $string['uploadthisfile'] = 'Upload this file'; $string['uploadusers'] = 'Upload users'; +$string['uploadnewfile'] = 'Upload new file'; +$string['uploadinvalidpreprocessedcount'] = 'Number of invalid preprocessed records: $a'; +$string['uploadfilecontainerror'] = 'The uploaded file has not been processed due to the error(s) specified. Please amend the file before uploading it again.'; $string['url'] = 'URL'; $string['used'] = 'Used'; $string['usedinnplaces'] = 'Used in $a places'; diff --git a/lib/moodlelib.php b/lib/moodlelib.php index 048425c9a6..c3e08a193e 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -219,6 +219,10 @@ define('PARAM_THEME', 'theme'); */ define('PARAM_URL', 'url'); +/** + * PARAM_USERNAME - Clean username to only contains specified characters. + */ +define('PARAM_USERNAME', 'username'); ///// DEPRECATED PARAM TYPES OR ALIASES - DO NOT USE FOR NEW CODE ///// @@ -463,6 +467,7 @@ function validate_param($param, $type, $allownull=false, $debuginfo='') { * @uses PARAM_BASE64 * @uses PARAM_TAG * @uses PARAM_SEQUENCE + * @uses PARAM_USERNAME * @param mixed $param the variable we are cleaning * @param int $type expected format of param after cleaning. * @return mixed @@ -722,6 +727,17 @@ function clean_param($param, $type) { return ''; // Specified theme is not installed } + case PARAM_USERNAME: + $param = str_replace(" " , "", $param); + if (empty($CFG->extendedusernamechars)) { + // regular expression, eliminate all chars EXCEPT: + // alphanum, dash (-), underscore (_), at sign (@) and period (.) characters. + $param = preg_replace('/[^-\.@_a-z0-9]/', '', $param); + } else { + $param = preg_replace('/[A-Z]/', '', $param); + } + return $param; + default: // throw error, switched parameters in optional_param or another serious problem print_error("unknownparamtype", '', '', $type); } diff --git a/lib/simpletest/testmoodlelib.php b/lib/simpletest/testmoodlelib.php index 2a7eb8162d..93f1654baf 100644 --- a/lib/simpletest/testmoodlelib.php +++ b/lib/simpletest/testmoodlelib.php @@ -266,6 +266,7 @@ class moodlelib_test extends UnitTestCase { * @uses PARAM_LOCALURL * @uses PARAM_CLEANHTML * @uses PARAM_SEQUENCE + * @uses PARAM_USERNAME * @param mixed $param the variable we are cleaning * @param int $type expected format of param after cleaning. * @return mixed @@ -295,6 +296,35 @@ class moodlelib_test extends UnitTestCase { $this->assertEqual(clean_param('/just/a/path', PARAM_LOCALURL), '/just/a/path'); $this->assertEqual(clean_param('funny:thing', PARAM_LOCALURL), ''); $this->assertEqual(clean_param('course/view.php?id=3', PARAM_LOCALURL), 'course/view.php?id=3'); + + $currentstatus = $CFG->extendedusernamechars; + + // Run tests with extended character == FALSE; + $CFG->extendedusernamechars = FALSE; + $this->assertEqual(clean_param('johndoe123', PARAM_USERNAME), 'johndoe123' ); + $this->assertEqual(clean_param('john.doe', PARAM_USERNAME), 'john.doe'); + $this->assertEqual(clean_param('john-doe', PARAM_USERNAME), 'john-doe'); + $this->assertEqual(clean_param('john- doe', PARAM_USERNAME), 'john-doe'); + $this->assertEqual(clean_param('john_doe', PARAM_USERNAME), 'john_doe'); + $this->assertEqual(clean_param('john@doe', PARAM_USERNAME), 'john@doe'); + $this->assertEqual(clean_param('john~doe', PARAM_USERNAME), 'johndoe'); + $this->assertEqual(clean_param('john´doe', PARAM_USERNAME), 'johndoe'); + $this->assertEqual(clean_param('john#$%&() ', PARAM_USERNAME), 'john'); + $this->assertEqual(clean_param('JOHNdóé ', PARAM_USERNAME), 'd'); + $this->assertEqual(clean_param('john.,:;-_/|\ñÑ[]A_X-,D {} ~!@#$%^&*()_+ ?><[] ščřžžý ?ýáž?žý??šdoe ', PARAM_USERNAME), 'john.-__-@_doe'); + + + // Test success condition, if extendedusernamechars == ENABLE; + $CFG->extendedusernamechars = TRUE; + $this->assertEqual(clean_param('john_doe', PARAM_USERNAME), 'john_doe'); + $this->assertEqual(clean_param('john@doe', PARAM_USERNAME), 'john@doe'); + $this->assertEqual(clean_param('john# $%&()+_^', PARAM_USERNAME), 'john#$%&()+_^'); + $this->assertEqual(clean_param('john~doe', PARAM_USERNAME), 'john~doe'); + $this->assertEqual(clean_param('joHN´doe', PARAM_USERNAME), 'jo´doe'); + $this->assertEqual(clean_param('johnDOE', PARAM_USERNAME), 'john'); + $this->assertEqual(clean_param('johndóé ', PARAM_USERNAME), 'johndóé'); + + $CFG->extendedusernamechars = $currentstatus; } function test_validate_param() { diff --git a/login/index.php b/login/index.php index 8fbd5d62f0..6d17064b76 100644 --- a/login/index.php +++ b/login/index.php @@ -105,7 +105,7 @@ if (empty($CFG->usesid) and $testcookies and (get_moodle_cookie() == '')) { / $frm->username = trim(moodle_strtolower($frm->username)); if (is_enabled_auth('none') && empty($CFG->extendedusernamechars)) { - $string = preg_replace("~[^(-\.[:alnum:])]~i", "", $frm->username); + $string = clean_param($frm->username, PARAM_USERNAME); if (strcmp($frm->username, $string)) { $errormsg = get_string('username').': '.get_string("alphanumerical"); $errorcode = 2; diff --git a/login/signup_form.php b/login/signup_form.php index da0865a530..2ce7ebbc55 100644 --- a/login/signup_form.php +++ b/login/signup_form.php @@ -88,8 +88,6 @@ class login_signup_form extends moodleform { function definition_after_data(){ $mform =& $this->_form; - - $mform->applyFilter('username', 'moodle_strtolower'); $mform->applyFilter('username', 'trim'); } @@ -102,10 +100,15 @@ class login_signup_form extends moodleform { if ($DB->record_exists('user', array('username'=>$data['username'], 'mnethostid'=>$CFG->mnet_localhost_id))) { $errors['username'] = get_string('usernameexists'); } else { - if (empty($CFG->extendedusernamechars)) { - $string = preg_replace("~[^(-\.[:alnum:])]~i", '', $data['username']); - if (strcmp($data['username'], $string)) { - $errors['username'] = get_string('alphanumerical'); + //check allowed characters + if ($data['username'] !== moodle_strtolower($data['username'])) { + $errors['username'] = get_string('usernamelowercase'); + } else { + if (empty($CFG->extendedusernamechars)) { + $string = clean_param($data['username'], PARAM_USERNAME); + if (strcmp($data['username'], $string)) { + $errors['username'] = get_string('invalidusername'); + } } } } diff --git a/user/editadvanced.php b/user/editadvanced.php index b8ff1f5d9d..eb2d37f5d5 100644 --- a/user/editadvanced.php +++ b/user/editadvanced.php @@ -63,7 +63,7 @@ if ($id == -1) { require_capability('moodle/user:create', $systemcontext); $user = new object(); $user->id = -1; - $user->auth = 'manual'; + $user->auth = 'manual'; $user->confirmed = 1; $user->deleted = 0; } else { @@ -135,17 +135,17 @@ if ($usernew = $userform->get_data()) { } else { $authplugin = get_auth_plugin($usernew->auth); } - - $usernew->username = trim($usernew->username); - $usernew->timemodified = time(); + + $usernew->username = clean_param($usernew->username, PARAM_USERNAME); + $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 unset($usernew->id); $usernew = file_postupdate_standard_editor($usernew, 'description', $editoroptions, null, 'user_profile', null); $usernew->mnethostid = $CFG->mnet_localhost_id; // always local user - $usernew->confirmed = 1; - $usernew->password = hash_internal_user_password($usernew->newpassword); + $usernew->confirmed = 1; + $usernew->password = hash_internal_user_password($usernew->newpassword); $usernew->id = $DB->insert_record('user', $usernew); $usercreated = true; @@ -223,7 +223,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 f7f435f295..1628ea7141 100644 --- a/user/editadvanced_form.php +++ b/user/editadvanced_form.php @@ -118,7 +118,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; @@ -147,9 +147,9 @@ class user_editadvanced_form extends moodleform { $err['username'] = get_string('usernamelowercase'); } else { if (empty($CFG->extendedusernamechars)) { - $string = preg_replace("/[^(-\.[:alnum:])]/i", '', $usernew->username); + $string = clean_param($usernew->username, PARAM_USERNAME); if ($usernew->username !== $string) { - $err['username'] = get_string('alphanumerical'); + $err['username'] = get_string('invalidusername'); } } } -- 2.39.5