MDL-64474 auth_oauth2: Properly update user profile data
authorJun Pataleta <jun@moodle.com>
Fri, 28 Dec 2018 08:50:12 +0000 (16:50 +0800)
committerJun Pataleta <jun@moodle.com>
Fri, 28 Dec 2018 08:52:37 +0000 (16:52 +0800)
* Updating of user profile data from OAuth2 issuer should only be
performed for fields that can be synced externally (fields defined in
\auth_plugin_base::$userfields)
* Only update user profile data for users which use OAuth2 as their
default authentication mechanism.

auth/oauth2/classes/auth.php

index 19cc83c..1ecfca8 100644 (file)
@@ -305,18 +305,54 @@ class auth extends \auth_plugin_base {
      * Update user data according to data sent by authorization server.
      *
      * @param array $externaldata data from authorization server
-     * @param int $userid ID of the user to update
-     * @return stdClass The updated user record
+     * @param stdClass $userdata Current data of the user to be updated
+     * @return stdClass The updated user record, or the existing one if there's nothing to be updated.
      */
-    private function update_user(array $externaldata, int $userid) {
+    private function update_user(array $externaldata, $userdata) {
         $user = (object) [
-            'id' => $userid,
+            'id' => $userdata->id,
         ];
+
+        // We can only update if the default authentication type of the user is set to OAuth2 as well. Otherwise, we might mess
+        // up the user data of other users that use different authentication mechanisms (e.g. linked logins).
+        if ($userdata->auth !== $this->authtype) {
+            return $userdata;
+        }
+
+        // Go through each field from the external data.
         foreach ($externaldata as $fieldname => $value) {
-            $user->$fieldname = $value;
+            if (!in_array($fieldname, $this->userfields)) {
+                // Skip if this field doesn't belong to the list of fields that can be synced with the OAuth2 issuer.
+                continue;
+            }
+
+            if (!property_exists($userdata, $fieldname)) {
+                // Just in case this field is on the list, but not part of the user data. This shouldn't happen though.
+                continue;
+            }
+
+            // Get the old value.
+            $oldvalue = (string)$userdata->$fieldname;
+
+            // Get the lock configuration of the field.
+            $lockvalue = $this->config->{'field_lock_' . $fieldname};
+
+            // We should update fields that meet the following criteria:
+            // - Lock value set to 'unlocked'; or 'unlockedifempty', given the current value is empty.
+            // - The value has changed.
+            if ($lockvalue === 'unlocked' || ($lockvalue === 'unlockedifempty' && empty($oldvalue))) {
+                $value = (string)$value;
+                if ($oldvalue !== $value) {
+                    $user->$fieldname = $value;
+                }
+            }
         }
+        // Update the user data.
         user_update_user($user, false);
 
+        // Save user profile data.
+        profile_save_data($user);
+
         // Refresh user for $USER variable.
         return get_complete_user_data('id', $user->id);
     }
@@ -439,7 +475,7 @@ class auth extends \auth_plugin_base {
                 redirect(new moodle_url('/login/index.php'));
             } else if ($mappeduser && $mappeduser->confirmed) {
                 // Update user fields.
-                $userinfo = $this->update_user($userinfo, $mappeduser->id);
+                $userinfo = $this->update_user($userinfo, $mappeduser);
                 $userwasmapped = true;
             } else {
                 // Trigger login failed event.
@@ -497,7 +533,7 @@ class auth extends \auth_plugin_base {
                     exit();
                 } else {
                     \auth_oauth2\api::link_login($userinfo, $issuer, $moodleuser->id, true);
-                    $userinfo = $this->update_user($userinfo, $moodleuser->id);
+                    $userinfo = $this->update_user($userinfo, $moodleuser);
                     // No redirect, we will complete this login.
                 }
 
@@ -562,8 +598,7 @@ class auth extends \auth_plugin_base {
                 } else {
                     // Create a new confirmed account.
                     $newuser = \auth_oauth2\api::create_new_confirmed_account($userinfo, $issuer);
-                    // Update new user's fields.
-                    $userinfo = $this->update_user($userinfo, $newuser->id);
+                    $userinfo = get_complete_user_data('id', $newuser->id);
                     // No redirect, we will complete this login.
                 }
             }