From c0721a6230748a54666a97f3b14f98398901a03d Mon Sep 17 00:00:00 2001
From: Jaime Perez Crespo <jaime.perez@uninett.no>
Date: Wed, 16 Mar 2016 13:50:42 +0100
Subject: [PATCH] Be graceful if a SAML assertion does not contain a NameID. Do
not set it in the state array, and set logout as "saml1" to avoid SLO, since
SLO requires NameIDs.
---
modules/saml/www/sp/saml2-acs.php | 45 ++++++++++++++++++++++---------
1 file changed, 32 insertions(+), 13 deletions(-)
diff --git a/modules/saml/www/sp/saml2-acs.php b/modules/saml/www/sp/saml2-acs.php
index e0e93415c..611ea0b08 100644
--- a/modules/saml/www/sp/saml2-acs.php
+++ b/modules/saml/www/sp/saml2-acs.php
@@ -186,23 +186,42 @@ if ($expire !== null) {
$logoutExpire = time() + 24 * 60 * 60;
}
-// register this session in the logout store
-sspmod_saml_SP_LogoutStore::addSession($sourceId, $nameId, $sessionIndex, $logoutExpire);
-
-// we need to save the NameID and SessionIndex for logout
-$logoutState = array(
- 'saml:logout:Type' => 'saml2',
- 'saml:logout:IdP' => $idp,
- 'saml:logout:NameID' => $nameId,
- 'saml:logout:SessionIndex' => $sessionIndex,
-);
+if (!empty($nameId)) {
+ // register this session in the logout store
+ sspmod_saml_SP_LogoutStore::addSession($sourceId, $nameId, $sessionIndex, $logoutExpire);
+
+ // we need to save the NameID and SessionIndex for logout
+ $logoutState = array(
+ 'saml:logout:Type' => 'saml2',
+ 'saml:logout:IdP' => $idp,
+ 'saml:logout:NameID' => $nameId,
+ 'saml:logout:SessionIndex' => $sessionIndex,
+ );
+
+ $state['saml:sp:NameID'] = $nameId; // no need to mark it as persistent, it already is
+} else {
+ /*
+ * No NameID provided, we can't logout from this IdP!
+ *
+ * Even though interoperability profiles "require" a NameID, the SAML 2.0 standard does not require it to be present
+ * in assertions. That way, we could have a Subject with only a SubjectConfirmation, or even no Subject element at
+ * all.
+ *
+ * In case we receive a SAML assertion with no NameID, we can be graceful and continue, but we won't be able to
+ * perform a Single Logout since the SAML logout profile mandates the use of a NameID to identify the individual we
+ * want to be logged out. In order to minimize the impact of this, we keep logout state information (without saving
+ * it to the store), marking the IdP as SAML 1.0, which does not implement logout. Then we can safely log the user
+ * out from the local session, skipping Single Logout upstream to the IdP.
+ */
+ $logoutState = array(
+ 'saml:logout:Type' => 'saml1',
+ );
+}
+
$state['LogoutState'] = $logoutState;
$state['saml:AuthenticatingAuthority'] = $authenticatingAuthority;
$state['saml:AuthenticatingAuthority'][] = $idp;
$state['PersistentAuthData'][] = 'saml:AuthenticatingAuthority';
-
-$state['saml:sp:NameID'] = $nameId;
-$state['PersistentAuthData'][] = 'saml:sp:NameID';
$state['saml:sp:SessionIndex'] = $sessionIndex;
$state['PersistentAuthData'][] = 'saml:sp:SessionIndex';
$state['saml:sp:AuthnContext'] = $assertion->getAuthnContext();
--
GitLab