berta initial commits

admin/tool/dataprivacy/tests/task/task_test.php

@@ -16,6 +16,7 @@
 
 namespace tool_dataprivacy\task;
 
+use core\task\task_trait;
 use tool_dataprivacy\api;
 
 defined('MOODLE_INTERNAL') || die();
@@ -30,6 +31,8 @@
  */
 class task_test extends \data_privacy_testcase {
 
+    use task_trait;
+
     /**
      * Test tearDown.
      */
@@ -215,17 +218,4 @@ public function test_delete_existing_deleted_users_task_existing_finished_delete
         $this->assertCount(1, \tool_dataprivacy\api::get_data_requests($user->id,
                 [api::DATAREQUEST_STATUS_CANCELLED], [api::DATAREQUEST_TYPE_DELETE]));
     }
-
-    /**
-     * Helper to execute a particular task.
-     *
-     * @param string $task The task.
-     */
-    private function execute_task($task) {
-        // Run the scheduled task.
-        ob_start();
-        $task = \core\task\manager::get_scheduled_task($task);
-        $task->execute();
-        ob_end_clean();
-    }
 }

admin/tool/filetypes/tests/behat/add_filetypes.feature

@@ -120,8 +120,7 @@ Feature: Add customised file types
       | Custom description | Froggy file                               |
     And I press "Save changes"
     # Create a resource activity and add it to a course
-    And I am on "Course 1" course homepage with editing mode on
-    When I add a "File" to section "1"
+    When I add a resource activity to course "Course 1" section "1"
     And I set the following fields to these values:
       | Name        | An example of customised file type |
       | Description | File description                   |

admin/tool/filetypes/tests/tool_filetypes_test.php

@@ -82,12 +82,24 @@ public function test_get_icons_from_path() {
     }
 
     /**
-     * Test get_file_icons() function to confirm no file icons are removed/added by mistake.
+     * Test get_file_icons() function to confirm no file icons are removed by mistake.
      *
      * @covers ::get_file_icons
      */
     public function test_get_file_icons() {
         $icons = utils::get_file_icons();
-        $this->assertCount(31, $icons);
+        $filetypes = core_filetypes::get_types();
+
+        $requiredicons = array_column($filetypes, 'icon');
+        $requireduniqueicons = array_unique($requiredicons);
+
+        // The 'folder' icon is not a file, however the test validates no
+        // file icons are removed by mistake from the directory pix/f.
+        // Adding the folder icon manually completes the scope of this test.
+        $requireduniqueicons[] = 'folder';
+
+        foreach ($requireduniqueicons as $requiredicon) {
+            $this->assertArrayHasKey($requiredicon, $icons, "Icon '$requiredicon' is missing.");
+        }
     }
 }

admin/tool/mfa/classes/output/renderer.php

@@ -333,8 +333,8 @@ public function factors_in_use_table(int $lookback): string {
 
         // Auth rows.
         $authtypes = get_enabled_auth_plugins(true);
-        $row = [];
         foreach ($authtypes as $authtype) {
+            $row = [];
             $row[] = \html_writer::tag('b', $authtype);
 
             // Setup the overall totals columns.

admin/tool/mobile/launch.php

@@ -56,8 +56,9 @@
 
 // Check if the plugin is properly configured.
 $typeoflogin = get_config('tool_mobile', 'typeoflogin');
-if (empty($SESSION->justloggedin) and
-        $typeoflogin != tool_mobile\api::LOGIN_VIA_BROWSER and
+if (empty($SESSION->justloggedin) &&
+        !is_enabled_auth('oauth2') &&
+        $typeoflogin != tool_mobile\api::LOGIN_VIA_BROWSER &&
         $typeoflogin != tool_mobile\api::LOGIN_VIA_EMBEDDED_BROWSER) {
     throw new moodle_exception('pluginnotenabledorconfigured', 'tool_mobile');
 }

admin/tool/phpunit/cli/util.php

@@ -50,12 +50,10 @@
     )
 );
 
-if (file_exists(__DIR__.'/../../../../vendor/phpunit/phpunit/composer.json')) {
-    // Composer packages present.
-    require_once(__DIR__.'/../../../../vendor/autoload.php');
-
-} else {
-    // Note: installation via PEAR is not supported any more.
+// Basic check to see if phpunit is installed.
+if (!file_exists(__DIR__.'/../../../../vendor/phpunit/phpunit/composer.json') ||
+        !file_exists(__DIR__.'/../../../../vendor/bin/phpunit') ||
+        !file_exists(__DIR__.'/../../../../vendor/autoload.php')) {
     phpunit_bootstrap_error(PHPUNIT_EXITCODE_PHPUNITMISSING);
 }
 
@@ -74,12 +72,13 @@
         }
     }
     $_SERVER['argv'] = array_values($_SERVER['argv']);
-    PHPUnit\TextUI\Command::main();
+    require(__DIR__ . '/../../../../vendor/bin/phpunit');
     exit(0);
 }
 
 define('PHPUNIT_UTIL', true);
 
+require(__DIR__.'/../../../../vendor/autoload.php');
 require(__DIR__ . '/../../../../lib/phpunit/bootstrap.php');
 
 // from now on this is a regular moodle CLI_SCRIPT

admin/tool/uploaduser/classes/process.php

@@ -461,9 +461,22 @@ public function process_line(array $line) {
             return;
         }
 
-        $matchparam = $this->get_match_on_email() ? ['email' => $user->email] : ['username' => $user->username];
-        if ($existinguser = $DB->get_records('user', $matchparam + ['mnethostid' => $user->mnethostid])) {
-            if (is_array($existinguser) && count($existinguser) !== 1) {
+        if ($this->get_match_on_email()) {
+            // Case-insensitive query for the given email address.
+            $userselect = $DB->sql_equal('email', ':email', false);
+            $userparams = ['email' => $user->email];
+        } else {
+            $userselect = 'username = :username';
+            $userparams = ['username' => $user->username];
+        }
+
+        // Match the user, also accounting for multiple records by email.
+        $existinguser = $DB->get_records_select('user', "{$userselect} AND mnethostid = :mnethostid",
+            $userparams + ['mnethostid' => $user->mnethostid]);
+        $existingusercount = count($existinguser);
+
+        if ($existingusercount > 0) {
+            if ($existingusercount !== 1) {
                 $this->upt->track('status', get_string('duplicateemail', 'tool_uploaduser', $user->email), 'warning');
                 $this->userserrors++;
                 return;

admin/tool/uploaduser/user_form.php

@@ -97,9 +97,6 @@ function definition () {
         $columns = $this->_customdata['columns'];
         $data    = $this->_customdata['data'];
 
-        // I am the template user, why should it be the administrator? we have roles now, other ppl may use this script ;-)
-        $templateuser = $USER;
-
         // upload settings and file
         $mform->addElement('header', 'settingsheader', get_string('settings'));
 
@@ -276,30 +273,19 @@ function definition () {
         $mform->setDefault('autosubscribe', core_user::get_property_default('autosubscribe'));
 
         $mform->addElement('text', 'city', get_string('city'), 'maxlength="120" size="25"');
-        $mform->setType('city', PARAM_TEXT);
-        if (empty($CFG->defaultcity)) {
-            $mform->setDefault('city', $templateuser->city);
-        } else {
-            $mform->setDefault('city', core_user::get_property_default('city'));
-        }
+        $mform->setType('city', core_user::get_property_type('city'));
+        $mform->setDefault('city', core_user::get_property_default('city'));
 
-        $choices = get_string_manager()->get_list_of_countries();
-        $choices = array(''=>get_string('selectacountry').'...') + $choices;
-        $mform->addElement('select', 'country', get_string('selectacountry'), $choices);
-        if (empty($CFG->country)) {
-            $mform->setDefault('country', $templateuser->country);
-        } else {
-            $mform->setDefault('country', core_user::get_property_default('country'));
-        }
+        $mform->addElement('select', 'country', get_string('selectacountry'), core_user::get_property_choices('country'));
+        $mform->setDefault('country', core_user::get_property_default('country') ?: '');
         $mform->setAdvanced('country');
 
-        $choices = core_date::get_list_of_timezones($templateuser->timezone, true);
-        $mform->addElement('select', 'timezone', get_string('timezone'), $choices);
-        $mform->setDefault('timezone', $templateuser->timezone);
+        $mform->addElement('select', 'timezone', get_string('timezone'), core_date::get_list_of_timezones(null, true));
+        $mform->setDefault('timezone', core_user::get_property_default('timezone'));
         $mform->setAdvanced('timezone');
 
-        $mform->addElement('select', 'lang', get_string('preferredlanguage'), get_string_manager()->get_list_of_translations());
-        $mform->setDefault('lang', $templateuser->lang);
+        $mform->addElement('select', 'lang', get_string('preferredlanguage'), core_user::get_property_choices('lang'));
+        $mform->setDefault('lang', core_user::get_property_default('lang'));
         $mform->setAdvanced('lang');
 
         $editoroptions = array('maxfiles'=>0, 'maxbytes'=>0, 'trusttext'=>false, 'forcehttps'=>false);
@@ -313,25 +299,23 @@ function definition () {
         $mform->setForceLtr('idnumber');
 
         $mform->addElement('text', 'institution', get_string('institution'), 'maxlength="255" size="25"');
-        $mform->setType('institution', PARAM_TEXT);
-        $mform->setDefault('institution', $templateuser->institution);
+        $mform->setType('institution', core_user::get_property_type('institution'));
 
         $mform->addElement('text', 'department', get_string('department'), 'maxlength="255" size="25"');
-        $mform->setType('department', PARAM_TEXT);
-        $mform->setDefault('department', $templateuser->department);
+        $mform->setType('department', core_user::get_property_type('department'));
 
         $mform->addElement('text', 'phone1', get_string('phone1'), 'maxlength="20" size="25"');
-        $mform->setType('phone1', PARAM_NOTAGS);
+        $mform->setType('phone1', core_user::get_property_type('phone1'));
         $mform->setAdvanced('phone1');
         $mform->setForceLtr('phone1');
 
         $mform->addElement('text', 'phone2', get_string('phone2'), 'maxlength="20" size="25"');
-        $mform->setType('phone2', PARAM_NOTAGS);
+        $mform->setType('phone2', core_user::get_property_type('phone2'));
         $mform->setAdvanced('phone2');
         $mform->setForceLtr('phone2');
 
         $mform->addElement('text', 'address', get_string('address'), 'maxlength="255" size="25"');
-        $mform->setType('address', PARAM_TEXT);
+        $mform->setType('address', core_user::get_property_type('address'));
         $mform->setAdvanced('address');
 
         // Next the profile defaults

auth/lti/auth.php

@@ -110,8 +110,7 @@ public function complete_login(array $launchdata, moodle_url $returnurl, int $pr
 
         // The platform user is already linked with a user account.
         if ($this->get_user_binding($launchdata['iss'], $launchdata['sub'])) {
-            // Always sync the PII, regardless of whether we're already authenticated as this user or not.
-            $user = $this->find_or_create_user_from_launch($launchdata, true);
+            $user = $this->find_or_create_user_from_launch($launchdata);
 
             if (isloggedin()) {
                 // If a different user is currently logged in, authenticate the linked user instead.
@@ -121,11 +120,13 @@ public function complete_login(array $launchdata, moodle_url $returnurl, int $pr
                 }
                 // If the linked user is already logged in, skip the call to complete_user_login() because this affects deep linking
                 // workflows on sites publishing and consuming resources on the same site, due to the regenerated sesskey.
-                return;
             } else {
                 complete_user_login($user);
-                return;
+
             }
+            // Always sync the PII, regardless of whether we're already authenticated as this user or not.
+            $this->update_user_account($user, $launchdata, $launchdata['iss']);
+            return;
         }
 
         // The platform user is not bound to a user account, check provisioning mode now.
@@ -136,7 +137,9 @@ public function complete_login(array $launchdata, moodle_url $returnurl, int $pr
         switch ($provisioningmode) {
             case self::PROVISIONING_MODE_AUTO_ONLY:
                 // Automatic provisioning - this will create/migrate a user account and log the user in.
-                complete_user_login($this->find_or_create_user_from_launch($launchdata, true, $legacyconsumersecrets));
+                $user = $this->find_or_create_user_from_launch($launchdata, $legacyconsumersecrets);
+                complete_user_login($user);
+                $this->update_user_account($user, $launchdata, $launchdata['iss']);
                 break;
             case self::PROVISIONING_MODE_PROMPT_NEW_EXISTING:
             case self::PROVISIONING_MODE_PROMPT_EXISTING_ONLY:
@@ -211,21 +214,13 @@ public function find_or_create_user_from_membership(array $member, string $iss,
      * itself and pass relevant data in - as auth_plugin_lti::complete_login() does.
      *
      * @param array $launchdata all data in the decoded JWT including iss and sub.
-     * @param bool $syncpicture whether to sync the user's picture with the picture sent in the launch.
      * @param array $legacyconsumersecrets all secrets found for the legacy consumer, facilitating user migration.
      * @return stdClass the Moodle user who is mapped to the platform user identified in the JWT data.
      */
-    public function find_or_create_user_from_launch(array $launchdata, bool $syncpicture = false,
-            array $legacyconsumersecrets = []): stdClass {
-
-        if (!$syncpicture) {
-            unset($launchdata['picture']);
-        }
+    public function find_or_create_user_from_launch(array $launchdata, array $legacyconsumersecrets = []): stdClass {
 
         if ($binduser = $this->get_user_binding($launchdata['iss'], $launchdata['sub'])) {
-            $user = \core_user::get_user($binduser);
-            $this->update_user_account($user, $launchdata, $launchdata['iss']);
-            return \core_user::get_user($user->id);
+            return \core_user::get_user($binduser);
         } else {
             // Is the intent to migrate a user account used in legacy launches?
             if (!empty($legacyconsumersecrets)) {
@@ -234,11 +229,10 @@ public function find_or_create_user_from_launch(array $launchdata, bool $syncpic
                     $usermigrationclaim = new user_migration_claim($launchdata, $legacyconsumersecrets);
                     $username = 'enrol_lti' .
                         sha1($usermigrationclaim->get_consumer_key() . '::' .
-                        $usermigrationclaim->get_consumer_key() .':' .$usermigrationclaim->get_user_id());
-                    if ($user = \core_user::get_user_by_username($username)) {
+                            $usermigrationclaim->get_consumer_key() . ':' . $usermigrationclaim->get_user_id());
+                    if ($user = core_user::get_user_by_username($username)) {
                         $this->create_user_binding($launchdata['iss'], $launchdata['sub'], $user->id);
-                        $this->update_user_account($user, $launchdata, $launchdata['iss']);
-                        return \core_user::get_user($user->id);
+                        return core_user::get_user($user->id);
                     }
                 } catch (Exception $e) {
                     // There was an issue validating the user migration claim. We don't want to fail auth entirely though.
@@ -247,9 +241,12 @@ public function find_or_create_user_from_launch(array $launchdata, bool $syncpic
                         "'{$launchdata['iss']}'. The migration claim could not be validated. A new account will be created.");
                 }
             }
+            // At the point of the creation, to ensure the user_created event correctly reflects the creating user of '0' (the user
+            // performing the action), ensure any active session is terminated and an empty session initialised.
+            $this->empty_session();
+
             $user = $this->create_new_account($launchdata, $launchdata['iss']);
-            $this->update_user_account($user, $launchdata, $launchdata['iss']);
-            return \core_user::get_user($user->id);
+            return core_user::get_user($user->id);
         }
     }
 
@@ -301,6 +298,17 @@ public function get_user_binding(string $issuer, string $sub): ?int {
         return $binduser;
     }
 
+    /**
+     * If there's an existing session, inits an empty session.
+     *
+     * @return void
+     */
+    protected function empty_session(): void {
+        if (isloggedin()) {
+            \core\session\manager::init_empty_session();
+        }
+    }
+
     /**
      * Check whether a provisioning mode is valid or not.
      *
@@ -361,7 +369,7 @@ protected function create_new_account(array $userdata, string $iss): stdClass {
      * @param array $userdata the user data coming from either a launch or membership service call.
      * @param string $iss the issuer to which the user belongs.
      */
-    protected function update_user_account(stdClass $user, array $userdata, string $iss): void {
+    public function update_user_account(stdClass $user, array $userdata, string $iss): void {
         global $CFG;
         require_once($CFG->dirroot.'/user/lib.php');
         if ($user->auth !== 'lti') {

auth/lti/login.php

@@ -72,8 +72,9 @@
     } else {
         // Create a new account and link it, logging the user in.
         $auth = get_auth_plugin('lti');
-        $newuser = $auth->find_or_create_user_from_launch($launchdata, true);
+        $newuser = $auth->find_or_create_user_from_launch($launchdata);
         complete_user_login($newuser);
+        $auth->update_user_account($newuser, $launchdata, $launchdata['iss']);
 
         $PAGE->set_context(context_system::instance());
         $PAGE->set_url(new moodle_url('/auth/lti/login.php'));