From 977857781a6c52c803dc472154d13adc017d708a Mon Sep 17 00:00:00 2001 From: James McQuillan Date: Wed, 3 Jul 2019 03:53:26 -0400 Subject: [PATCH] MSFTMPP-765: Improve resiliency of auth_oidc_token userid linking --- classes/loginflow/authcode.php | 27 +++++++++++++++++++-------- classes/observers.php | 4 ++-- lang/en/auth_oidc.php | 1 + 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/classes/loginflow/authcode.php b/classes/loginflow/authcode.php index 226fd5a..a446cfd 100644 --- a/classes/loginflow/authcode.php +++ b/classes/loginflow/authcode.php @@ -416,15 +416,18 @@ class authcode extends \auth_oidc\loginflow\base { $tokenrec = $DB->get_record('auth_oidc_token', ['oidcuniqid' => $oidcuniqid]); if (!empty($tokenrec)) { // Already connected user. - if (empty($tokenrec->userid)) { - // ERROR. - echo 'ERROR1';die(); + // ERROR1 + throw new \moodle_exception('exception_tokenemptyuserid', 'auth_oidc'); } $user = $DB->get_record('user', ['id' => $tokenrec->userid]); if (empty($user)) { - // ERROR. - echo 'ERROR2';die(); + // ERROR2 + $failurereason = AUTH_LOGIN_NOUSER; + $eventdata = ['other' => ['username' => $username, 'reason' => $failurereason]]; + $event = \core\event\user_login_failed::create($eventdata); + $event->trigger(); + throw new \moodle_exception('errorauthloginfailednouser', 'auth_oidc', null, null, '1'); } $username = $user->username; $this->updatetoken($tokenrec->id, $authparams, $tokenparams); @@ -473,14 +476,22 @@ class authcode extends \auth_oidc\loginflow\base { $user = authenticate_user_login($username, null, true); if (!empty($user)) { + $tokenrec = $DB->get_record('auth_oidc_token', ['id' => $tokenrec->id]); + // This should be already done in auth_plugin_oidc::user_authenticated_hook, but just in case... + if (!empty($tokenrec) && empty($tokenrec->userid)) { + $updatedtokenrec = new \stdClass; + $updatedtokenrec->id = $tokenrec->id; + $updatedtokenrec->userid = $user->id; + $DB->update_record('auth_oidc_token', $updatedtokenrec); + } complete_user_login($user); return true; } else { + // There was a problem in authenticate_user_login. Clean up incomplete token record. if (!empty($tokenrec)) { - throw new \moodle_exception('errorlogintoconnectedaccount', 'auth_oidc', null, null, '2'); - } else { - throw new \moodle_exception('errorauthloginfailednouser', 'auth_oidc', null, null, '2'); + $DB->delete_records('auth_oidc_token', ['id' => $tokenrec->id]); } + throw new \moodle_exception('errorauthgeneral', 'auth_oidc', null, null, '2'); } return true; diff --git a/classes/observers.php b/classes/observers.php index bc52802..11bcef9 100644 --- a/classes/observers.php +++ b/classes/observers.php @@ -37,8 +37,8 @@ class observers { */ public static function handle_user_deleted(\core\event\user_deleted $event) { global $DB; - $eventdata = $event->get_data(); - $DB->delete_records('auth_oidc_token', ['username' => $eventdata['other']['username']]); + $userid = $event->objectid; + $DB->delete_records('auth_oidc_token', ['userid' => $userid]); return true; } } diff --git a/lang/en/auth_oidc.php b/lang/en/auth_oidc.php index acbfb55..76aba7c 100644 --- a/lang/en/auth_oidc.php +++ b/lang/en/auth_oidc.php @@ -120,6 +120,7 @@ $string['eventusercreated'] = 'User created with OpenID Connect'; $string['eventuserconnected'] = 'User connected to OpenID Connect'; $string['eventuserloggedin'] = 'User Logged In with OpenID Connect'; $string['eventuserdisconnected'] = 'User disconnected from OpenID Connect'; +$string['exception_tokenemptyuserid'] = 'The existing token for this user does not contain a valid user ID. Please contact your administrator.'; $string['oidc:manageconnection'] = 'Allow OpenID Connection and Disconnection'; $string['oidc:manageconnectionconnect'] = 'Allow OpenID Connection';