From db4f7811dfdf51e07f72a8f37d2cb281e84a1b4f Mon Sep 17 00:00:00 2001
From: Jaime Perez <jaime.perez@uninett.no>
Date: Fri, 27 Jun 2014 16:15:16 +0200
Subject: [PATCH] Fix #82 and #83. Make the checks for an IDPList happen only
when we are the IdP authenticating the user, not an intermediate proxy.
---
lib/SimpleSAML/IdP.php | 2 --
modules/saml/lib/Auth/Source/SP.php | 55 +++++++++++++++++++++++++++++
2 files changed, 55 insertions(+), 2 deletions(-)
diff --git a/lib/SimpleSAML/IdP.php b/lib/SimpleSAML/IdP.php
index d9b997dc6..119231a00 100644
--- a/lib/SimpleSAML/IdP.php
+++ b/lib/SimpleSAML/IdP.php
@@ -379,8 +379,6 @@ class SimpleSAML_IdP {
if (isset($state['ForceAuthn']) && (bool)$state['ForceAuthn']) {
/* Force authentication is in effect. */
$needAuth = TRUE;
- } elseif (isset($state['saml:IDPList']) && sizeof($state['saml:IDPList']) > 0) {
- $needAuth = !in_array($this->authSource->getAuthData('saml:sp:IdP'), $state['saml:IDPList'], TRUE);
} else {
$needAuth = !$this->isAuthenticated();
}
diff --git a/modules/saml/lib/Auth/Source/SP.php b/modules/saml/lib/Auth/Source/SP.php
index a2b223588..a23cb0fe9 100644
--- a/modules/saml/lib/Auth/Source/SP.php
+++ b/modules/saml/lib/Auth/Source/SP.php
@@ -392,6 +392,61 @@ class sspmod_saml_Auth_Source_SP extends SimpleSAML_Auth_Source {
}
+ /**
+ * Re-authenticate an user.
+ *
+ * This function is called by the IdP to give the authentication source a chance to
+ * interact with the user even in the case when the user is already authenticated.
+ *
+ * @param array &$state Information about the current authentication.
+ */
+ public function reauthenticate(array &$state) {
+ assert('is_array($state)');
+
+ $session = SimpleSAML_Session::getInstance();
+ $data = $session->getAuthState($this->authId);
+ foreach ($data as $k => $v) {
+ $state[$k] = $v;
+ }
+
+ // check if we have an IDPList specified in the request
+ if (isset($state['saml:IDPList']) && sizeof($state['saml:IDPList']) > 0 &&
+ !in_array($state['saml:sp:IdP'], $state['saml:IDPList'], TRUE)) {
+ /*
+ * This is essentially wrong. The IdP used to authenticate the current session is not in the IDPList
+ * that we just received, so we are triggering authentication again against an IdP in the IDPList. This
+ * is fine if the user wants to, but we SHOULD offer the user to logout before proceeding.
+ *
+ * After successful authentication in a different IdP, the reauthPostLogin callback will be invoked,
+ * overriding the current session with a new one, associated with the new IdP. This will leave us in an
+ * inconsistent state, with several service providers with valid sessions they got from different IdPs.
+ *
+ * TODO: we need to offer the user the possibility to logout before blindly authenticating him again.
+ */
+ $state['LoginCompletedHandler'] = array('sspmod_saml_Auth_Source_SP', 'reauthPostLogin');
+ $this->authenticate($state);
+ }
+ }
+
+
+ /**
+ * Complete login operation after re-authenticating the user on another IdP.
+ *
+ * @param array $state The authentication state.
+ */
+ public static function reauthPostLogin(array $state) {
+ assert('isset($state["ReturnCallback"])');
+
+ // Update session state
+ $session = SimpleSAML_Session::getInstance();
+ $session->doLogin($state['saml:sp:AuthId'], SimpleSAML_Auth_Default::extractPersistentAuthState($state));
+
+ // resume the login process
+ call_user_func($state['ReturnCallback'], $state);
+ assert('FALSE');
+ }
+
+
/**
* Start a SAML 2 logout operation.
*
--
GitLab