Skip to content

Commit

Permalink
MDL-64474 auth_oauth2: Properly update user profile data
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
junpataleta committed Dec 28, 2018
1 parent 38a1b4f commit e2b812f
Showing 1 changed file with 44 additions and 9 deletions.
53 changes: 44 additions & 9 deletions auth/oauth2/classes/auth.php
Original file line number Diff line number Diff line change
Expand Up @@ -305,18 +305,54 @@ private function update_picture($user) {
* 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);
}
Expand Down Expand Up @@ -439,7 +475,7 @@ public function complete_login(client $client, $redirecturl) {
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.
Expand Down Expand Up @@ -497,7 +533,7 @@ public function complete_login(client $client, $redirecturl) {
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.
}

Expand Down Expand Up @@ -562,8 +598,7 @@ public function complete_login(client $client, $redirecturl) {
} 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.
}
}
Expand Down

0 comments on commit e2b812f

Please sign in to comment.