Merge pull request #565 from kabalin/fix-logout

Fix warning on logout.

Author: danmarsden

TESTING.md

@@ -18,7 +18,8 @@ http://moodle.local/auth/saml2/login.php?wants&idp=c4b9265e38e3107bee1ccdf9d6475
 http://moodle.local/login/logout.php?sesskey=ihwmEywPxu
 
 
-4) Test Single logout starting from the IdP
+4) Test Single logout starting from the IdP. Notice that `ReturnTo` URL domain should be in `trusted.url.domains` in IdP config.
+If that is not the case, try using `ReturnTo=http://idp.local/simplesaml` which should work as SimpleSAMLphp trusts self hostname by default.
 
 http://idp.local/simplesaml/saml2/idp/SingleLogoutService.php?ReturnTo=http://moodle.local/
 

classes/api.php

@@ -29,17 +29,19 @@
  * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 class api {
+
     /**
-     * Called from SimpleSamlphp after a LogoutResponse from the IdP
+     * IdP logout callback. Called only when logout is initiated from IdP.
+     * {@see saml2-logout.php}
      */
     public static function logout_from_idp_front_channel(): void {
-        // The SP session will be cleaned up but we need to remove the
-        // Moodle session here.
-        \core\session\manager::terminate_current();
+        // The SP session will be cleaned up. Log user out of Moodle.
+        require_logout();
     }
 
     /**
-     * Called from SimpleSamlphp after a LogoutRequest from the SP
+     * SP logout callback. Called in case of normal Moodle logout.
+     * {@see auth::logoutpage_hook}
      *
      * @param array $state Information about the current logout operation.
      */

sp/saml2-logout.php

@@ -33,10 +33,11 @@
 /*
  * There are 4 methods of logging out:
  *
- * 1) Initiated from moodles logout, in which case we first logout of
+ * 1) Initiated from moodle logout, in which case we first logout of
  *    moodle and then log out of the middle SP and then optionally
  *    redirect to the IdP to do full Single Logout. This is the way
- *    a majority of users logout and is fully supported.
+ *    a majority of users logout and is fully supported. Notice that in this
+ *    case SAML session is not authenticated by the time we reach this point.
  *
  * 2) If doing SLO via IdP via the HTTP-Redirect binding
  *
@@ -48,7 +49,12 @@
  */
 try {
     $session = \SimpleSAML\Session::getSessionFromRequest();
-    $session->registerLogoutHandler($saml2auth->spname, '\auth_saml2\api', 'logout_from_idp_front_channel');
+    // When logout is initiated from IdP (we land here from SingleLogoutService call),
+    // session is still authenticated, so we can register the handler that will log
+    // user out in Moodle.
+    if (!is_null($session->getAuthState($saml2auth->spname))) {
+        $session->registerLogoutHandler($saml2auth->spname, '\auth_saml2\api', 'logout_from_idp_front_channel');
+    }
 
     require('../.extlib/simplesamlphp/modules/saml/www/sp/saml2-logout.php');
 } catch (Exception $e) {