MDL-16919 we have to really use the username cleaning only when manua…

…lly adding new accounts, any sync with external system needs the exact match without any cleaning!
moodle · Jun 6, 2010 · 6b8ad96 · 6b8ad96
1 parent 621b4d0
commit 6b8ad96
Show file tree

Hide file tree

Showing 12 changed files with 21 additions and 61 deletions.
diff --git a/auth/cas/auth.php b/auth/cas/auth.php
@@ -98,7 +98,7 @@ function loginpage_hook() {
       $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 @@ function sync_users ($do_updates = true) {
                     $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
@@ -364,8 +364,7 @@ function sync_users($do_updates=false) {
                     $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
@@ -73,13 +73,12 @@ function can_signup() {
      * @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
@@ -419,7 +419,6 @@ function user_signup($user, $notify=true) {
             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 @@ function sync_users($do_updates=true) {
                     $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
@@ -270,7 +270,6 @@ function confirm_mnet_session($token, $remotepeer) {
             $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
@@ -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
@@ -306,7 +306,6 @@ function enrol_user($user, $courseid) {
              */
             $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
@@ -225,7 +225,7 @@
 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);
  * </code>
  *
- * @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
@@ -105,8 +105,7 @@
     $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
@@ -106,12 +106,11 @@ function validation($data, $files) {
             //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
@@ -62,7 +62,7 @@
     // 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 @@
     } 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 @@
         $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 @@
             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
@@ -114,7 +114,7 @@ function definition_after_data() {
         profile_definition_after_data($mform, $userid);
     }
 
-    function validation($usernew, $files) { 
+    function validation($usernew, $files) {
         global $CFG, $DB;
 
         $usernew = (object)$usernew;
@@ -141,9 +141,8 @@ function validation($usernew, $files) {
             //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');
                 }
             }