From 56383be366576b0d2597857f8797aa741f9cc48f Mon Sep 17 00:00:00 2001 From: Jakob Schlupmann Date: Sat, 9 Oct 2021 13:48:33 +0200 Subject: [PATCH 1/2] SOAP binding / backend logout work in progress proposal --- .../simplesamlphp/lib/SimpleSAML/Session.php | 6 +++++- .../modules/saml/www/sp/saml2-logout.php | 4 +++- classes/api.php | 13 +++++++++++++ sp/saml2-logout.php | 19 +++++++++++++++++++ 4 files changed, 40 insertions(+), 2 deletions(-) diff --git a/.extlib/simplesamlphp/lib/SimpleSAML/Session.php b/.extlib/simplesamlphp/lib/SimpleSAML/Session.php index 0c9c6c508..b1682a1ee 100644 --- a/.extlib/simplesamlphp/lib/SimpleSAML/Session.php +++ b/.extlib/simplesamlphp/lib/SimpleSAML/Session.php @@ -736,7 +736,11 @@ private function callLogoutHandlers(string $authority): void assert(isset($this->authData[$authority])); if (empty($this->authData[$authority]['LogoutHandlers'])) { - return; + if (empty($this->authData[$authority]['Attributes']['username'][0])) { + return; + } else { + call_user_func(['\auth_saml2\api', 'logout_from_idp_back_channel'], $this->authData[$authority]['Attributes']['username'][0], $this->sessionId); + } } foreach ($this->authData[$authority]['LogoutHandlers'] as $handler) { // verify that the logout handler is a valid function diff --git a/.extlib/simplesamlphp/modules/saml/www/sp/saml2-logout.php b/.extlib/simplesamlphp/modules/saml/www/sp/saml2-logout.php index 9b756d8d6..25ad8a7ef 100644 --- a/.extlib/simplesamlphp/modules/saml/www/sp/saml2-logout.php +++ b/.extlib/simplesamlphp/modules/saml/www/sp/saml2-logout.php @@ -130,6 +130,7 @@ $dst = $idpMetadata->getEndpointPrioritizedByBinding( 'SingleLogoutService', [ + \SAML2\Constants::BINDING_SOAP, \SAML2\Constants::BINDING_HTTP_REDIRECT, \SAML2\Constants::BINDING_HTTP_POST ] @@ -143,8 +144,9 @@ $dst = $dst['Location']; } $binding->setDestination($dst); + } else { + $lr->setDestination($dst['Location']); } - $lr->setDestination($dst); $binding->send($lr); } else { diff --git a/classes/api.php b/classes/api.php index ab522f80a..e4d5974c7 100644 --- a/classes/api.php +++ b/classes/api.php @@ -39,6 +39,19 @@ public static function logout_from_idp_front_channel(): void { require_logout(); } + public static function logout_from_idp_back_channel($username, $sessionId): void { + global $DB; + if (isset($sessionId)) { + $DB->delete_records('auth_saml2_kvstore', array('k' => $sessionId)); + } + + $userid = $DB->get_record('user', array('username' => $username), 'id'); + $mdsessionids = $DB->get_records('sessions', array('userid' => $userid->id), 'sid DESC', 'sid'); + foreach ($mdsessionids as $mdsessionid) { + $DB->delete_records('sessions', array('sid' => $mdsessionid->sid)); + } + } + /** * SP logout callback. Called in case of normal Moodle logout. * {@see auth::logoutpage_hook} diff --git a/sp/saml2-logout.php b/sp/saml2-logout.php index 6777e4162..7253ab2cd 100644 --- a/sp/saml2-logout.php +++ b/sp/saml2-logout.php @@ -54,6 +54,25 @@ // user out in Moodle. if (!is_null($session->getAuthState($saml2auth->spname))) { $session->registerLogoutHandler($saml2auth->spname, '\auth_saml2\api', 'logout_from_idp_front_channel'); + } else { + try { + $binding = \SAML2\Binding::getCurrentBinding(); + } catch (\Exception $e) { + // TODO: look for a specific exception + // This is dirty. Instead of checking the message of the exception, \SAML2\Binding::getCurrentBinding() should throw + // an specific exception when the binding is unknown, and we should capture that here + if ($e->getMessage() === 'Unable to find the current binding.') { + throw new \SimpleSAML\Error\Error('SLOSERVICEPARAMS', $e, 400); + } else { + throw $e; // do not ignore other exceptions! + } + } + $message = $binding->receive(); + + if ($message instanceof \SAML2\LogoutRequest) { + //Todo : parse xmlbody, get sp sessionid/moodle userid, and register logoutHandler from here ? + //For the time being the call to the specific logout function is done with 3 lines in extlib.. session.php + } } require('../.extlib/simplesamlphp/modules/saml/www/sp/saml2-logout.php'); From 8d4bf12533b4a40942846cb99605407f3b86cf82 Mon Sep 17 00:00:00 2001 From: Jakob Schlupmann Date: Sun, 12 Dec 2021 23:05:46 +0100 Subject: [PATCH 2/2] Revised backchannel logout proposal --- .../simplesamlphp/lib/SimpleSAML/Session.php | 10 +-- classes/api.php | 17 ++-- classes/auth.php | 6 ++ sp/saml2-logout.php | 78 +++++++++++++++---- 4 files changed, 80 insertions(+), 31 deletions(-) diff --git a/.extlib/simplesamlphp/lib/SimpleSAML/Session.php b/.extlib/simplesamlphp/lib/SimpleSAML/Session.php index b1682a1ee..6167517a3 100644 --- a/.extlib/simplesamlphp/lib/SimpleSAML/Session.php +++ b/.extlib/simplesamlphp/lib/SimpleSAML/Session.php @@ -736,12 +736,9 @@ private function callLogoutHandlers(string $authority): void assert(isset($this->authData[$authority])); if (empty($this->authData[$authority]['LogoutHandlers'])) { - if (empty($this->authData[$authority]['Attributes']['username'][0])) { - return; - } else { - call_user_func(['\auth_saml2\api', 'logout_from_idp_back_channel'], $this->authData[$authority]['Attributes']['username'][0], $this->sessionId); - } + return; } + foreach ($this->authData[$authority]['LogoutHandlers'] as $handler) { // verify that the logout handler is a valid function if (!is_callable($handler)) { @@ -750,10 +747,9 @@ private function callLogoutHandlers(string $authority): void throw new \Exception( 'Logout handler is not a valid function: ' . $classname . '::' . - $functionname + $functionname ); } - // call the logout handler call_user_func($handler); } diff --git a/classes/api.php b/classes/api.php index e4d5974c7..2de001dfb 100644 --- a/classes/api.php +++ b/classes/api.php @@ -39,17 +39,16 @@ public static function logout_from_idp_front_channel(): void { require_logout(); } - public static function logout_from_idp_back_channel($username, $sessionId): void { - global $DB; - if (isset($sessionId)) { - $DB->delete_records('auth_saml2_kvstore', array('k' => $sessionId)); - } + public static function logout_from_idp_back_channel(): void + { + global $DB, $sp_sessionId; - $userid = $DB->get_record('user', array('username' => $username), 'id'); - $mdsessionids = $DB->get_records('sessions', array('userid' => $userid->id), 'sid DESC', 'sid'); - foreach ($mdsessionids as $mdsessionid) { - $DB->delete_records('sessions', array('sid' => $mdsessionid->sid)); + if (isset($sp_sessionId)) { + $DB->delete_records('auth_saml2_kvstore', array('k' => $sp_sessionId)); + $session = \SimpleSAML\Session::getSession($sp_sessionId); + \core\session\manager::kill_session($session->moodle_session_id); } + } /** diff --git a/classes/auth.php b/classes/auth.php index 9bc49196e..32c822d5b 100644 --- a/classes/auth.php +++ b/classes/auth.php @@ -726,6 +726,12 @@ public function saml_login_complete($attributes) { $USER->site = $CFG->wwwroot; set_moodle_cookie($USER->username); + $moodle_session_id = session_id(); + $saml_session = \SimpleSAML\Session::getSessionFromRequest(); + $saml_session->moodle_session_id = $moodle_session_id; + $saml_session_handler = \SimpleSAML\SessionHandler::getSessionHandler(); + $saml_session_handler->saveSession($saml_session); + $wantsurl = core_login_get_return_url(); // If we are not on the page we want, then redirect to it (unless this is CLI). if ( qualified_me() !== false && qualified_me() !== $wantsurl ) { diff --git a/sp/saml2-logout.php b/sp/saml2-logout.php index 7253ab2cd..40a90f29e 100644 --- a/sp/saml2-logout.php +++ b/sp/saml2-logout.php @@ -54,24 +54,72 @@ // user out in Moodle. if (!is_null($session->getAuthState($saml2auth->spname))) { $session->registerLogoutHandler($saml2auth->spname, '\auth_saml2\api', 'logout_from_idp_front_channel'); - } else { + } else { + // check if binding message exists and is logout request try { - $binding = \SAML2\Binding::getCurrentBinding(); - } catch (\Exception $e) { - // TODO: look for a specific exception - // This is dirty. Instead of checking the message of the exception, \SAML2\Binding::getCurrentBinding() should throw - // an specific exception when the binding is unknown, and we should capture that here - if ($e->getMessage() === 'Unable to find the current binding.') { - throw new \SimpleSAML\Error\Error('SLOSERVICEPARAMS', $e, 400); - } else { - throw $e; // do not ignore other exceptions! + $binding = \SAML2\Binding::getCurrentBinding(); + } catch (\Exception $e) { + // TODO: look for a specific exception + // This is dirty. Instead of checking the message of the exception, \SAML2\Binding::getCurrentBinding() should throw + // an specific exception when the binding is unknown, and we should capture that here + if ($e->getMessage() === 'Unable to find the current binding.') { + throw new \SimpleSAML\Error\Error('SLOSERVICEPARAMS', $e, 400); + } else { + throw $e; // do not ignore other exceptions! + } + } + $message = $binding->receive(); + if ($message instanceof \SAML2\LogoutRequest) { + $nameId = $message->getNameId(); + $sessionIndexes = $message->getSessionIndexes(); + + // Getting session from $nameId and $sessionIndexes + $authId = $saml2auth->spname; + + assert(is_string($authId)); + + $store = \SimpleSAML\Store::getInstance(); + if ($store === false) { + // We don't have a datastore + // TODO throw error } - } - $message = $binding->receive(); + + // serialize and anonymize the NameID + $strNameId = serialize($nameId); + $strNameId = sha1($strNameId); - if ($message instanceof \SAML2\LogoutRequest) { - //Todo : parse xmlbody, get sp sessionid/moodle userid, and register logoutHandler from here ? - //For the time being the call to the specific logout function is done with 3 lines in extlib.. session.php + // Normalize SessionIndexes + foreach ($sessionIndexes as &$sessionIndex) { + assert(is_string($sessionIndex)); + if (strlen($sessionIndex) > 50) { + $sessionIndex = sha1($sessionIndex); + } + } + + // Remove reference + unset($sessionIndex); + + if ($store instanceof \SimpleSAML\Store\SQL) { + // TODO : ssp_sessions stored in db option + //$sessions = self::getSessionsSQL($store, $authId, $strNameId); + } else { + if (empty($sessionIndexes)) { + // We cannot fetch all sessions without a SQL store + return false; + } + + foreach ($sessionIndexes as $sessionIndex) { + $sessionId = $store->get('saml.LogoutStore', $strNameId . ':' . $sessionIndex); + if ($sessionId === null) { + continue; + } + assert(is_string($sessionId)); + $session = \SimpleSAML\Session::getSession($sessionId); + $session->registerLogoutHandler($authId, '\auth_saml2\api', 'logout_from_idp_back_channel'); + $sp_sessionId = $sessionId; + continue; // only registering first session... + } + } } }