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 | 121 ++++++++++++++++++++++++------- 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, 188 insertions(+), 46 deletions(-) diff --git a/admin/uploaduser.php b/admin/uploaduser.php index b8dce1997b1..54073eb3612 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; +$cir->init(); +$filerows = array(); +while ($fields = $cir->next()) { + $ci = 0; + if ($ri > $previewrows) { + $arow .= ''; + foreach ($fields as $field) { + $arow .= '...'; + } + $arow .= ''; + break; + } + + $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 .= ''; + $arow .= '
'.s($field). '
'; + $arow .= ''; + + if ($key == (count($fields) - 1) && !empty($errormsg)) { + $errormsg = implode('
', $errormsg); + $arow .= '' . $errormsg . ''; + $filerows[$ri]['error'] = $errormsg; + } + } + $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 ''; echo ''; foreach ($columns as $col) { echo ''; } +if (!empty($invalidcontent)) { + echo ''; +} echo ''; -$cir->init(); -while ($fields = $cir->next()) { - if ($ri > $previewrows) { - echo ''; - foreach ($fields as $field) { - echo '';; - } - break; - } - $ci = 0; - echo ''; - foreach ($fields as $field) { - echo '';; - } - echo ''; +$countcontent = 0; +if (empty($invalidcontent)) { + $countcontent = count($validcontent); + echo implode('', $validcontent); +} else { + $countcontent = count($invalidcontent); + echo implode('', $invalidcontent); } -$cir->close(); - echo '
'.s($col).''.moodle_strtolower(get_string('error', 'moodle')).'
...
'.s($field).'
'; -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 19da95500a9..2525927e08c 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 b30b32fa63d..e1f147569aa 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 441911cb1e7..57c1ebf75f3 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 a1bb6e4ee8c..f6e82042690 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 2de694cb1e0..33a17f4d533 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 41a79adff56..a03a8db6b56 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 96cfbf13a48..2f70acc50b5 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 6ce416390f1..a541c5d824f 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 c3303911964..60bc3cc20d7 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 53c99562b47..47bc55cc521 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 048425c9a63..c3e08a193ec 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 2a7eb8162d0..93f1654bafb 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 8fbd5d62f00..6d17064b76a 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 da0865a5307..2ce7ebbc55c 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 b8ff1f5d9d0..eb2d37f5d59 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 f7f435f2952..1628ea7141b 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'); } } }