Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-52781

Improve core_user::fill_properties_cache()

    XMLWordPrintable

    Details

    • Testing Instructions:
      Hide

      This issue is adding new validation to core user functions to create and update user data, so it must test all types of user data input, from authentication plugins sync scripts, since moodle mobile.

      Pre-requisites
      1. Install a new instance with this patch applied or clone integration branch.
      2. Go through the install paying attention to any debug message or errors.
      3. When you reach the point to add your admin user, try to hack and bypass the form validation.
      4. Make sure you don't see any error
      5. Go to Site administration ► Development ► Debugging
      6. Set debug messages to DEVELOPER.
      7. Enable debugdisplay.
      8. Make sure your PHP config has error_log setted to a valid file.
        • If not setted, add a valid file to be your php error log.
      Upload user tool
      1. For this test, you might need a CSV file with few users.
      2. Edit your CSV file, adding some malicious code to some user fields.
      3. - For example firstname - Eric <script>alert(1);</script>Cartman
      4. Test other validations changing fields content to something invalid:
        • timezone - Australia_perth
        • lang - ww
        • country - AUSTRALIA
      5. Set your default system settings for timezone, lang, country (you can try other settings, calendartype for example)
      6. Visit Site administration ► Users ► Accounts ► Upload users
      7. Choose your CSV file and click Upload users button.
      8. You might see debugging messages for those fields with invalid data in the UI and in your logs, something like this:

        [20-Apr-2016 20:56:52 America/Mexico_City] PHP Notice:  The property 'name of the field here' has invalid data and has been cleaned.<ul style="text-align: left" data-rel="backtrace"><li>line 95 of /user/lib.php: call to debugging()</li><li>line 818 of /admin/tool/uploaduser/index.php: call to user_create_user()</li></ul> in /home/moodle/moodles/stable_master/moodle/lib/weblib.php on line 3014

      9. It should display a warning for each users that has invalid data on the upload results.
      10. Check on the user table, if the user has been added and if the fields with invalid data got cleaned and in case of country and others, it setted the default system setting.
      11. Try again, now change you csv file, and add valid user data (valid country, languages..timezones).
        • timezone - America/Mexico City
        • lang - en
        • country - AU
      12. You should not see any error or debugging message in your logs.
      Webservices
      1. Create users using the webservices: core_user_create_users and core_user_update_users
      2. Make sure you can create users and the webservice is not broken.
      3. Would be nice to extend a bit more and create and update users, using moodle mobile.
      4. You should be able to add and update users as expected.
      External DB auth plugin.
      1. Enable and setup external db plugin.
        • You can simply dump the user table structure and restore in a different database.
      2. Add few users to the external db user table.
      3. Run the sync: php auth/db/cli/sync_users.php
      4. Make sure all users in the external db got created on your moodle user table.
      5. Now, change user data accordingly to the upload users tool example.
      6. You should get debugging messages and your user invalid data should be cleaned.
      LDAP & CAS
      1. As CAS uses LDAP user sync, both can be tested at same time.
      2. Pretty much the same as above, you should be able to import users without any problem.
        • Invalid data should be displayed in the debugging message.
        • Your users invalid data should be cleaned.
      Email auth
      1. Enable and setup email authentication.
      2. Make sure it works as expected.
      PHP Unit and Behat
      1. All php unit tests and behat should pass (check nightly results).
      Show
      This issue is adding new validation to core user functions to create and update user data, so it must test all types of user data input, from authentication plugins sync scripts, since moodle mobile. Pre-requisites Install a new instance with this patch applied or clone integration branch. Go through the install paying attention to any debug message or errors. When you reach the point to add your admin user, try to hack and bypass the form validation. Make sure you don't see any error Go to Site administration ► Development ► Debugging Set debug messages to DEVELOPER . Enable debugdisplay. Make sure your PHP config has error_log setted to a valid file. If not setted, add a valid file to be your php error log. Upload user tool For this test, you might need a CSV file with few users. Edit your CSV file, adding some malicious code to some user fields. - For example firstname - Eric <script>alert(1);</script>Cartman Test other validations changing fields content to something invalid: timezone - Australia_perth lang - ww country - AUSTRALIA Set your default system settings for timezone, lang, country (you can try other settings, calendartype for example) Visit Site administration ► Users ► Accounts ► Upload users Choose your CSV file and click Upload users button. You might see debugging messages for those fields with invalid data in the UI and in your logs, something like this: [20-Apr-2016 20:56:52 America/Mexico_City] PHP Notice: The property 'name of the field here' has invalid data and has been cleaned.<ul style="text-align: left" data-rel="backtrace"><li>line 95 of /user/lib.php: call to debugging()</li><li>line 818 of /admin/tool/uploaduser/index.php: call to user_create_user()</li></ul> in /home/moodle/moodles/stable_master/moodle/lib/weblib.php on line 3014 It should display a warning for each users that has invalid data on the upload results. Check on the user table, if the user has been added and if the fields with invalid data got cleaned and in case of country and others, it setted the default system setting. Try again, now change you csv file, and add valid user data (valid country, languages..timezones). timezone - America/Mexico City lang - en country - AU You should not see any error or debugging message in your logs . Webservices Create users using the webservices: core_user_create_users and core_user_update_users Make sure you can create users and the webservice is not broken. Would be nice to extend a bit more and create and update users, using moodle mobile . You should be able to add and update users as expected. External DB auth plugin. Enable and setup external db plugin. You can simply dump the user table structure and restore in a different database. Add few users to the external db user table. Run the sync: php auth/db/cli/sync_users.php Make sure all users in the external db got created on your moodle user table. Now, change user data accordingly to the upload users tool example. You should get debugging messages and your user invalid data should be cleaned. LDAP & CAS As CAS uses LDAP user sync, both can be tested at same time. Pretty much the same as above, you should be able to import users without any problem. Invalid data should be displayed in the debugging message. Your users invalid data should be cleaned. Email auth Enable and setup email authentication. Make sure it works as expected. PHP Unit and Behat All php unit tests and behat should pass (check nightly results).
    • Affected Branches:
      MOODLE_31_STABLE
    • Fixed Branches:
      MOODLE_31_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-52781-master
    • Sprint:
      3.1 Sprint 7

      Description

      On the MDL-50705 we've introduced the core_user::fill_properties_cache() to be a reference of which parameter should be used to validate a specific field of the user table.
      In the first stage, only the type property was added, now, we need to improve that method, adding other properties null and choices.

      • The null property would be a reference to check if a user field is NULL_ALLOWED or NULL_NOT_ALLOWED.
      • The choices property would control which values are accepted for a user field, for example: timezones, country, calendartype, themes... all those options should be checked agains a list of accepted values. In the timezone for example should be a valid timezone, checked against core_date::get_list_of_timezones(). The country should be checked against get_string_manager()->get_list_of_countries() and so on..

      The main goal of this issue is sanitize the data inserted on moodle and make sure it's safe.
      Things that should be considered on this issue:

      • Add a validate() method, to validate a field against a parameter.
      • Maybe add a clean() method, to clean the data.
      • Decide if this validation should be extended to user API methods: user_create_user() and user_update_user.
      • Consider change other entry points such webservices and forms(good experiment would be to change the normal and advanced user edit forms) to use this new parameter validation.
      • Extensive testing instructions to make user it doesn't break anything.

        Attachments

          Issue Links

            Activity

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                9 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  23/May/16