From c70e0b7564acd0dbf54d9ac805a92ca024c98212 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jaime=20Pe=CC=81rez?= <jaime.perez@uninett.no>
Date: Mon, 15 Aug 2016 15:49:05 +0200
Subject: [PATCH] Multiple enhancements and fixes to IDPList support in proxy
mode.
- Bugfix: the modules/saml/www/proxy/invalid_session.php shouldn't call directly the error handler in sspmod_saml_IdP_SAML2. Instead, it should use the SimpleSAML_Auth_State::throwException() method to let it handle the exception appropriately (in this case, it should always return back to the requester).
- The standard specifies that a "urn:oasis:names:tc:SAML:2.0:status:NoSupportedIDP" or "urn:oasis:names:tc:SAML:2.0:status:NoAvailableIDP" second-level status code should be returned to the requester in case an error occurs. Add a couple of exceptions to represent both statuses, and use them to set the right status code in the response.
- We shouldn't ask the user to logout in case the IDPList does not offer an IdP we recognize, or in case the proxy enforces the use of an IdP ('idp' configuration option in the auth source) and such IdP is in the IDPList.
- Similarly, these two cases should also handled in case we are authenticating for the first time, not only when reauthenticating.
---
lib/SimpleSAML/Error/NoAvailableIDP.php | 15 +++++++
lib/SimpleSAML/Error/NoSupportedIDP.php | 15 +++++++
modules/saml/lib/Auth/Source/SP.php | 46 +++++++++++++++++++---
modules/saml/lib/Error.php | 17 +++++++-
modules/saml/www/proxy/invalid_session.php | 4 +-
5 files changed, 88 insertions(+), 9 deletions(-)
create mode 100644 lib/SimpleSAML/Error/NoAvailableIDP.php
create mode 100644 lib/SimpleSAML/Error/NoSupportedIDP.php
diff --git a/lib/SimpleSAML/Error/NoAvailableIDP.php b/lib/SimpleSAML/Error/NoAvailableIDP.php
new file mode 100644
index 000000000..d3c99dc23
--- /dev/null
+++ b/lib/SimpleSAML/Error/NoAvailableIDP.php
@@ -0,0 +1,15 @@
+<?php
+/**
+ * Simple exception to model the NoAvailableIDP SAML error.
+ *
+ * @author Jaime PĂ©rez Crespo, UNINETT AS <jaime.perez@uninett.no>
+ * @package SimpleSAMLphp
+ */
+
+namespace SimpleSAML\Error;
+
+
+class NoAvailableIDP extends \SimpleSAML_Error_Exception
+{
+
+}
\ No newline at end of file
diff --git a/lib/SimpleSAML/Error/NoSupportedIDP.php b/lib/SimpleSAML/Error/NoSupportedIDP.php
new file mode 100644
index 000000000..0ef97ed2a
--- /dev/null
+++ b/lib/SimpleSAML/Error/NoSupportedIDP.php
@@ -0,0 +1,15 @@
+<?php
+/**
+ * Simple exception to model the NoSupportedIDP SAML error.
+ *
+ * @author Jaime PĂ©rez Crespo, UNINETT AS <jaime.perez@uninett.no>
+ * @package SimpleSAMLphp
+ */
+
+namespace SimpleSAML\Error;
+
+
+class NoSupportedIDP extends \SimpleSAML_Error_Exception
+{
+
+}
\ No newline at end of file
diff --git a/modules/saml/lib/Auth/Source/SP.php b/modules/saml/lib/Auth/Source/SP.php
index 4a091fdb5..99827f59c 100644
--- a/modules/saml/lib/Auth/Source/SP.php
+++ b/modules/saml/lib/Auth/Source/SP.php
@@ -385,8 +385,23 @@ class sspmod_saml_Auth_Source_SP extends SimpleSAML_Auth_Source {
$idp = (string)$state['saml:idp'];
}
- if ($idp === NULL && isset($state['saml:IDPList']) && sizeof($state['saml:IDPList']) == 1) {
- $idp = $state['saml:IDPList'][0];
+ if (isset($state['saml:IDPList']) && sizeof($state['saml:IDPList']) > 0) {
+ // we have a SAML IDPList (we are a proxy): filter the list of IdPs available
+ $mdh = SimpleSAML_Metadata_MetaDataStorageHandler::getMetadataHandler();
+ $known_idps = $mdh->getList();
+ $intersection = array_intersect($state['saml:IDPList'], array_keys($known_idps));
+
+ if (empty($intersection)) { // all requested IdPs are unknown
+ throw new SimpleSAML\Error\NoSupportedIDP('None of the IdPs requested are supported by this proxy.');
+ }
+
+ if (!is_null($idp) && !in_array($idp, $intersection)) { // the IdP is enforced but not in the IDPList
+ throw new SimpleSAML\Error\NoAvailableIDP('None of the IdPs requested are available to this proxy.');
+ }
+
+ if (is_null($idp) && sizeof($intersection) === 1) { // only one IdP requested or valid
+ $idp = current($state['saml:IDPList']);
+ }
}
if ($idp === NULL) {
@@ -422,9 +437,30 @@ class sspmod_saml_Auth_Source_SP extends SimpleSAML_Auth_Source {
{
/*
* The user has an existing, valid session. However, the SP provided a list of IdPs it accepts for
- * authentication, and the IdP the existing session is related to is not in that list. We need to
- * inform the user, and ask whether we should logout before starting the authentication process again
- * with a different IdP, or cancel the current SSO attempt.
+ * authentication, and the IdP the existing session is related to is not in that list.
+ *
+ * First, check if we recognize any of the IdPs requested.
+ */
+
+ $mdh = SimpleSAML_Metadata_MetaDataStorageHandler::getMetadataHandler();
+ $known_idps = $mdh->getList();
+ $intersection = array_intersect($state['saml:IDPList'], array_keys($known_idps));
+
+ if (empty($intersection)) { // all requested IdPs are unknown
+ throw new SimpleSAML\Error\NoSupportedIDP('None of the IdPs requested are supported by this proxy.');
+ }
+
+ /*
+ * We have at least one IdP in the IDPList that we recognize, and it's not the one currently in use. Let's
+ * see if this proxy enforces the use of one single IdP.
+ */
+ if (!is_null($this->idp) && !in_array($this->idp, $intersection)) { // an IdP is enforced but not requested
+ throw new SimpleSAML\Error\NoAvailableIDP('None of the IdPs requested are available to this proxy.');
+ }
+
+ /*
+ * We need to inform the user, and ask whether we should logout before starting the authentication process
+ * again with a different IdP, or cancel the current SSO attempt.
*/
SimpleSAML\Logger::warning(
"Reauthentication after logout is needed. The IdP '${state['saml:sp:IdP']}' is not in the IDPList ".
diff --git a/modules/saml/lib/Error.php b/modules/saml/lib/Error.php
index 78799a2cd..8cf0fb15f 100644
--- a/modules/saml/lib/Error.php
+++ b/modules/saml/lib/Error.php
@@ -117,8 +117,21 @@ class sspmod_saml_Error extends SimpleSAML_Error_Exception {
\SAML2\Constants::STATUS_PROXY_COUNT_EXCEEDED,
$exception->getMessage(),
$exception
- );
-
+ );
+ } elseif ($exception instanceof SimpleSAML\Error\NoAvailableIDP) {
+ $e = new self(
+ \SAML2\Constants::STATUS_RESPONDER,
+ \SAML2\Constants::STATUS_NO_AVAILABLE_IDP,
+ $exception->getMessage(),
+ $exception
+ );
+ } elseif ($exception instanceof SimpleSAML\Error\NoSupportedIDP) {
+ $e = new self(
+ \SAML2\Constants::STATUS_RESPONDER,
+ \SAML2\Constants::STATUS_NO_SUPPORTED_IDP,
+ $exception->getMessage(),
+ $exception
+ );
} else {
$e = new self(
\SAML2\Constants::STATUS_RESPONDER,
diff --git a/modules/saml/www/proxy/invalid_session.php b/modules/saml/www/proxy/invalid_session.php
index ac38a65d7..5369ace8a 100644
--- a/modules/saml/www/proxy/invalid_session.php
+++ b/modules/saml/www/proxy/invalid_session.php
@@ -28,8 +28,8 @@ try {
if (isset($_POST['cancel'])) {
// the user does not want to logout, cancel login
- $e = new SimpleSAML_Error_Exception('User refused to reauthenticate with any of the IdPs requested.');
- sspmod_saml_IdP_SAML2::handleAuthError($e, $state);
+ $e = new \SimpleSAML\Error\NoAvailableIDP('User refused to reauthenticate with any of the IdPs requested.');
+ SimpleSAML_Auth_State::throwException($state, $e);
}
if (isset($_POST['continue'])) {
--
GitLab