Fix warning on logout.

Before registering a logout handler check that session is still
authenticated. If logout initiated from Moodle, the session will be
destroyed earlier, so no need to register a handler (we logged out user
in Moodle already anyway). When logout is initiated from IdP, the SP
session will be active, so we register handler that will perform logout
in Moodle when session is destroyed by SP.

Closes #520

Author: kabalin

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) {