MSFTMPP-765: Improve resiliency of auth_oidc_token userid linking

microsoft · Jul 29, 2019 · 9778577 · 9778577
1 parent 199a24c
commit 9778577
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 10 deletions.
diff --git a/classes/loginflow/authcode.php b/classes/loginflow/authcode.php
@@ -416,15 +416,18 @@ protected function handlelogin($oidcuniqid, $authparams, $tokenparams, $idtoken)
         $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 @@ protected function handlelogin($oidcuniqid, $authparams, $tokenparams, $idtoken)
             $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
@@ -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
@@ -120,6 +120,7 @@
 $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';