From 519968c659d96d0f23f6551c8d3b9423ecfb332c Mon Sep 17 00:00:00 2001
From: Patrick <patrick@cirrusidentity.com>
Date: Tue, 11 Jun 2019 14:05:13 -0700
Subject: [PATCH] Allow list of entity ids to be loaded to better support
 idpList (#1138)

* Allow list of entity ids to be loaded to better support idpList

Certain metadata sources, like mdq, were don't support loading all metadata which
prevented them from being used with idplist. This change adds support for allowing
metadata sources to load a list of entities, allowing MDQ to be used for idp list.
---
 composer.json                                 |   2 +-
 .../Metadata/MetaDataStorageHandler.php       |  33 ++++
 .../MetaDataStorageHandlerSerialize.php       |  12 ++
 .../Metadata/MetaDataStorageSource.php        |  40 ++++-
 lib/SimpleSAML/Metadata/Sources/MDQ.php       |  12 ++
 modules/saml/lib/Auth/Source/SP.php           |  11 +-
 .../Metadata/MetaDataStorageHandlerTest.php   |  47 ++++++
 .../Metadata/MetaDataStorageSourceTest.php    |  61 ++++++-
 .../source1/saml20-sp-remote.php              |  60 +++++++
 .../saml20-sp-remote/entityB.serialized       |   1 +
 .../saml20-sp-remote/entityInBoth.serialized  |   1 +
 .../expiredInSrc1InSrc2.serialized            |   1 +
 .../lib/Auth/Source/Auth_Source_SP_Test.php   | 159 +++++++++++++++++-
 13 files changed, 422 insertions(+), 18 deletions(-)
 create mode 100644 tests/lib/SimpleSAML/Metadata/MetaDataStorageHandlerTest.php
 create mode 100644 tests/lib/SimpleSAML/Metadata/test-metadata/source1/saml20-sp-remote.php
 create mode 100644 tests/lib/SimpleSAML/Metadata/test-metadata/source2/saml20-sp-remote/entityB.serialized
 create mode 100644 tests/lib/SimpleSAML/Metadata/test-metadata/source2/saml20-sp-remote/entityInBoth.serialized
 create mode 100644 tests/lib/SimpleSAML/Metadata/test-metadata/source2/saml20-sp-remote/expiredInSrc1InSrc2.serialized

diff --git a/composer.json b/composer.json
index ecbc271d0..007e580d3 100644
--- a/composer.json
+++ b/composer.json
@@ -27,7 +27,7 @@
     },
     "autoload-dev": {
         "psr-4": {
-            "SimpleSAML\\Test\\": "tests"
+            "SimpleSAML\\Test\\": ["tests", "tests/lib/SimpleSAML"]
         },
         "files": ["tests/_autoload_modules.php"]
     },
diff --git a/lib/SimpleSAML/Metadata/MetaDataStorageHandler.php b/lib/SimpleSAML/Metadata/MetaDataStorageHandler.php
index 00b5674be..9c1476709 100644
--- a/lib/SimpleSAML/Metadata/MetaDataStorageHandler.php
+++ b/lib/SimpleSAML/Metadata/MetaDataStorageHandler.php
@@ -8,6 +8,8 @@ use SimpleSAML\Configuration;
 use SimpleSAML\Error;
 use SimpleSAML\Logger;
 use SimpleSAML\Utils;
+use SimpleSAML\Error\MetadataNotFound;
+use SimpleSAML\Utils\ClearableState;
 
 /**
  * This file defines a class for metadata handling.
@@ -260,6 +262,37 @@ class MetaDataStorageHandler implements \SimpleSAML\Utils\ClearableState
         return null;
     }
 
+    /**
+     * This function loads the metadata for entity IDs in $entityIds. It is returned as an associative array
+     * where the key is the entity id. An empty array may be returned if no matching entities were found
+     * @param array $entityIds The entity ids to load
+     * @param string $set The set we want to get metadata from.
+     * @return array An associative array with the metadata for the requested entities, if found.
+     */
+    public function getMetaDataForEntities(array $entityIds, $set)
+    {
+        $result = [];
+        foreach ($this->sources as $source) {
+            $srcList = $source->getMetaDataForEntities($entityIds, $set);
+            foreach ($srcList as $key => $le) {
+                if (array_key_exists('expire', $le)) {
+                    if ($le['expire'] < time()) {
+                        unset($srcList[$key]);
+                        \SimpleSAML\Logger::warning(
+                            "Dropping metadata entity ".var_export($key, true).", expired ".
+                            \SimpleSAML\Utils\Time::generateTimestamp($le['expire'])."."
+                        );
+                        continue;
+                    }
+                }
+                // We found the entity id so remove it from the list that needs resolving
+                unset($entityIds[array_search($key, $entityIds)]);
+            }
+            $result = array_merge($srcList, $result);
+        }
+
+        return $result;
+    }
 
     /**
      * This function looks up the metadata for the given entity id in the given set. It will throw an
diff --git a/lib/SimpleSAML/Metadata/MetaDataStorageHandlerSerialize.php b/lib/SimpleSAML/Metadata/MetaDataStorageHandlerSerialize.php
index 0d7474e3d..a41663fcc 100644
--- a/lib/SimpleSAML/Metadata/MetaDataStorageHandlerSerialize.php
+++ b/lib/SimpleSAML/Metadata/MetaDataStorageHandlerSerialize.php
@@ -290,4 +290,16 @@ class MetaDataStorageHandlerSerialize extends MetaDataStorageSource
             );
         }
     }
+
+    /**
+     * This function loads the metadata for entity IDs in $entityIds. It is returned as an associative array
+     * where the key is the entity id. An empty array may be returned if no matching entities were found
+     * @param array $entityIds The entity ids to load
+     * @param string $set The set we want to get metadata from.
+     * @return array An associative array with the metadata for the requested entities, if found.
+     */
+    public function getMetaDataForEntities(array $entityIds, $set)
+    {
+        return $this->getMetaDataForEntitiesIndividually($entityIds, $set);
+    }
 }
diff --git a/lib/SimpleSAML/Metadata/MetaDataStorageSource.php b/lib/SimpleSAML/Metadata/MetaDataStorageSource.php
index 81ffd3b12..3ec6c0b55 100644
--- a/lib/SimpleSAML/Metadata/MetaDataStorageSource.php
+++ b/lib/SimpleSAML/Metadata/MetaDataStorageSource.php
@@ -57,7 +57,7 @@ abstract class MetaDataStorageSource
      *
      * @param array $sourceConfig Associative array with the configuration for this metadata source.
      *
-     * @return mixed An instance of a metadata source with the given configuration.
+     * @return \SimpleSAML\Metadata\MetaDataStorageSource An instance of a metadata source with the given configuration.
      *
      * @throws \Exception If the metadata source type is invalid.
      */
@@ -245,6 +245,44 @@ abstract class MetaDataStorageSource
         return null;
     }
 
+    /**
+     * This function loads the metadata for entity IDs in $entityIds. It is returned as an associative array
+     * where the key is the entity id. An empty array may be returned if no matching entities were found.
+     * Subclasses should override if their getMetadataSet returns nothing or is slow. Subclasses may want to
+     * delegate to getMetaDataForEntitiesIndividually if loading entities one at a time is faster.
+     * @param array $entityIds The entity ids to load
+     * @param string $set The set we want to get metadata from.
+     * @return array An associative array with the metadata for the requested entities, if found.
+     */
+    public function getMetaDataForEntities(array $entityIds, $set)
+    {
+        if (count($entityIds) === 1) {
+            return $this->getMetaDataForEntitiesIndividually($entityIds, $set);
+        }
+        $entities = $this->getMetadataSet($set);
+        return array_intersect_key($entities, array_flip($entityIds));
+    }
+
+    /**
+     * Loads metadata entities one at a time, rather than the default implementation of loading all entities
+     * and filtering.
+     * @see MetaDataStorageSource::getMetaDataForEntities()
+     * @param array $entityIds The entity ids to load
+     * @param string $set The set we want to get metadata from.
+     * @return array An associative array with the metadata for the requested entities, if found.
+     */
+    protected function getMetaDataForEntitiesIndividually(array $entityIds, $set)
+    {
+        $entities = [];
+        foreach ($entityIds as $entityId) {
+            $metadata = $this->getMetaData($entityId, $set);
+            if ($metadata !== null) {
+                $entities[$entityId] = $metadata;
+            }
+        }
+        return $entities;
+    }
+
     /**
      * This method returns the full metadata set for a given entity id or null if the entity id cannot be found
      * in the given metadata set.
diff --git a/lib/SimpleSAML/Metadata/Sources/MDQ.php b/lib/SimpleSAML/Metadata/Sources/MDQ.php
index bd47dfb09..399975d5d 100644
--- a/lib/SimpleSAML/Metadata/Sources/MDQ.php
+++ b/lib/SimpleSAML/Metadata/Sources/MDQ.php
@@ -346,4 +346,16 @@ class MDQ extends \SimpleSAML\Metadata\MetaDataStorageSource
 
         return $data;
     }
+
+    /**
+     * This function loads the metadata for entity IDs in $entityIds. It is returned as an associative array
+     * where the key is the entity id. An empty array may be returned if no matching entities were found
+     * @param array $entityIds The entity ids to load
+     * @param string $set The set we want to get metadata from.
+     * @return array An associative array with the metadata for the requested entities, if found.
+     */
+    public function getMetaDataForEntities(array $entityIds, $set)
+    {
+        return $this->getMetaDataForEntitiesIndividually($entityIds, $set);
+    }
 }
diff --git a/modules/saml/lib/Auth/Source/SP.php b/modules/saml/lib/Auth/Source/SP.php
index b055e8b23..6aa40efe0 100644
--- a/modules/saml/lib/Auth/Source/SP.php
+++ b/modules/saml/lib/Auth/Source/SP.php
@@ -780,10 +780,9 @@ class SP extends \SimpleSAML\Auth\Source
         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 = MetaDataStorageHandler::getMetadataHandler();
-            $known_idps = $mdh->getList();
-            $intersection = array_intersect($state['saml:IDPList'], array_keys($known_idps));
+            $matchedEntities = $mdh->getMetaDataForEntities($state['saml:IDPList'], 'saml20-idp-remote');
 
-            if (empty($intersection)) {
+            if (empty($matchedEntities)) {
                 // all requested IdPs are unknown
                 throw new Module\saml\Error\NoSupportedIDP(
                     Constants::STATUS_REQUESTER,
@@ -791,7 +790,7 @@ class SP extends \SimpleSAML\Auth\Source
                 );
             }
 
-            if (!is_null($idp) && !in_array($idp, $intersection, true)) {
+            if (!is_null($idp) && !array_key_exists($idp, $matchedEntities)) {
                 // the IdP is enforced but not in the IDPList
                 throw new Module\saml\Error\NoAvailableIDP(
                     Constants::STATUS_REQUESTER,
@@ -799,9 +798,9 @@ class SP extends \SimpleSAML\Auth\Source
                 );
             }
 
-            if (is_null($idp) && sizeof($intersection) === 1) {
+            if (is_null($idp) && sizeof($matchedEntities) === 1) {
                 // only one IdP requested or valid
-                $idp = current($state['saml:IDPList']);
+                $idp = key($matchedEntities);
             }
         }
 
diff --git a/tests/lib/SimpleSAML/Metadata/MetaDataStorageHandlerTest.php b/tests/lib/SimpleSAML/Metadata/MetaDataStorageHandlerTest.php
new file mode 100644
index 000000000..d54c5c8e5
--- /dev/null
+++ b/tests/lib/SimpleSAML/Metadata/MetaDataStorageHandlerTest.php
@@ -0,0 +1,47 @@
+<?php
+
+namespace SimpleSAML\Test\Metadata;
+
+use SimpleSAML\Configuration;
+use SimpleSAML\Metadata\MetaDataStorageHandler;
+use SimpleSAML\Test\Utils\ClearStateTestCase;
+
+class MetaDataStorageHandlerTest extends ClearStateTestCase
+{
+
+    /**
+     * Test that loading specific entities works, and that metadata source precedence is followed
+     */
+    public function testLoadEntities()
+    {
+        $c = [
+            'metadata.sources' => [
+                ['type' => 'flatfile', 'directory' => __DIR__ . '/test-metadata/source1'],
+                ['type' => 'serialize', 'directory' => __DIR__ . '/test-metadata/source2'],
+            ],
+        ];
+        Configuration::loadFromArray($c, '', 'simplesaml');
+        $handler = MetaDataStorageHandler::getMetadataHandler();
+
+        $entities = $handler->getMetaDataForEntities([
+            'entityA',
+            'entityB',
+            'nosuchEntity',
+            'entityInBoth',
+            'expiredInSrc1InSrc2'
+        ], 'saml20-sp-remote');
+        $this->assertCount(4, $entities);
+        $this->assertEquals('entityA SP from source1', $entities['entityA']['name']['en']);
+        $this->assertEquals('entityB SP from source2', $entities['entityB']['name']['en']);
+        $this->assertEquals(
+            'entityInBoth SP from source1',
+            $entities['entityInBoth']['name']['en'],
+            "Entity is in both sources, but should get loaded from the first"
+        );
+        $this->assertEquals(
+            'expiredInSrc1InSrc2 SP from source2',
+            $entities['expiredInSrc1InSrc2']['name']['en'],
+            "Entity is in both sources, expired in src1 and available from src2"
+        );
+    }
+}
diff --git a/tests/lib/SimpleSAML/Metadata/MetaDataStorageSourceTest.php b/tests/lib/SimpleSAML/Metadata/MetaDataStorageSourceTest.php
index 6134676c0..372e6f944 100644
--- a/tests/lib/SimpleSAML/Metadata/MetaDataStorageSourceTest.php
+++ b/tests/lib/SimpleSAML/Metadata/MetaDataStorageSourceTest.php
@@ -2,6 +2,8 @@
 
 namespace SimpleSAML\Test\Metadata;
 
+use SimpleSAML\Configuration;
+
 /**
  * Class MetaDataStorageSourceTest
  */
@@ -36,8 +38,54 @@ class MetaDataStorageSourceTest extends \PHPUnit\Framework\TestCase
     public function testStaticXMLSource()
     {
         $testEntityId = "https://saml.idp/entityid";
+        $strTestXML = self::generateIdpMetadataXml($testEntityId);
+        // The primary test here is that - in contrast to the others above - this loads without error
+        // As a secondary thing, check that the entity ID from the static source provided can be extracted
+        $source = \SimpleSAML\Metadata\MetaDataStorageSource::getSource(["type"=>"xml", "xml"=>$strTestXML]);
+        $idpSet = $source->getMetadataSet("saml20-idp-remote");
+        $this->assertArrayHasKey($testEntityId, $idpSet, "Did not extract expected IdP entity ID from static XML source");
+        // Finally verify that a different entity ID does not get loaded
+        $this->assertCount(1, $idpSet, "Unexpectedly got metadata for an alternate entity than that defined");
+    }
+
+    /**
+     * Test loading multiple entities
+     */
+    public function testLoadEntitiesStaticXMLSource()
+    {
+        $c = [
+            'key' => 'value'
+        ];
+        Configuration::loadFromArray($c, '', 'simplesaml');
+        $entityId1 = "https://example.com";
+        $xml1 = self::generateIdpMetadataXml($entityId1);
+        $entityId2 = "https://saml.idp/entity";
+        $xml2 = self::generateIdpMetadataXml($entityId2);
         $strTestXML = "
-<EntityDescriptor ID=\"_12345678-90ab-cdef-1234-567890abcdef\" entityID=\"$testEntityId\" xmlns=\"urn:oasis:names:tc:SAML:2.0:metadata\">
+        <EntitiesDescriptor xmlns=\"urn:oasis:names:tc:SAML:2.0:metadata\">
+        $xml1
+        $xml2
+        </EntitiesDescriptor>
+        ";
+        $source = \SimpleSAML\Metadata\MetaDataStorageSource::getSource(["type"=>"xml", "xml"=>$strTestXML]);
+        // search that is a single entity
+        $entities = $source->getMetaDataForEntities([$entityId2], "saml20-idp-remote");
+        $this->assertCount(1, $entities, 'Only 1 entity loaded');
+        $this->assertArrayHasKey($entityId2, $entities);
+        // search for multiple entities
+        $entities = $source->getMetaDataForEntities([$entityId1, 'no-such-entity', $entityId2], "saml20-idp-remote");
+        $this->assertCount(2, $entities, 'Only 2 of the entities are found');
+        $this->assertArrayHasKey($entityId1, $entities);
+        $this->assertArrayHasKey($entityId2, $entities);
+        // search for non-existant entities
+        $entities = $source->getMetaDataForEntities(['no-such-entity'], "saml20-idp-remote");
+        $this->assertCount(0, $entities, 'no matches expected');
+    }
+
+    public static function generateIdpMetadataXml($entityId)
+    {
+        return "
+<EntityDescriptor ID=\"_12345678-90ab-cdef-1234-567890abcdef\" entityID=\"$entityId\" xmlns=\"urn:oasis:names:tc:SAML:2.0:metadata\">
 <RoleDescriptor xsi:type=\"fed:ApplicationServiceType\"
 protocolSupportEnumeration=\"http://docs.oasis-open.org/ws-sx/ws-trust/200512 http://schemas.xmlsoap.org/ws/2005/02/trust http://docs.oasis-open.org/wsfed/federation/200706\"
 ServiceDisplayName=\"SimpleSAMLphp Test\"
@@ -47,15 +95,10 @@ xmlns:fed=\"http://docs.oasis-open.org/wsfed/federation/200706\">
 <SingleSignOnService Binding=\"urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect\" Location=\"https://saml.idp/sso/\"/>
 <SingleLogoutService Binding=\"urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect\" Location=\"https://saml.idp/logout/\"/>
 </RoleDescriptor>
-<IDPSSODescriptor protocolSupportEnumeration=\"urn:oasis:names:tc:SAML:2.0:protocol\"/>
+<IDPSSODescriptor protocolSupportEnumeration=\"urn:oasis:names:tc:SAML:2.0:protocol\">
+<SingleSignOnService Binding=\"urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect\" Location=\"https://saml.idp/sso/\"/>
+</IDPSSODescriptor>
 </EntityDescriptor>
 ";
-        // The primary test here is that - in contrast to the others above - this loads without error
-        // As a secondary thing, check that the entity ID from the static source provided can be extracted
-        $source = \SimpleSAML\Metadata\MetaDataStorageSource::getSource(["type"=>"xml", "xml"=>$strTestXML]);
-        $idpSet = $source->getMetadataSet("saml20-idp-remote");
-        $this->assertArrayHasKey($testEntityId, $idpSet, "Did not extract expected IdP entity ID from static XML source");
-        // Finally verify that a different entity ID does not get loaded
-        $this->assertCount(1, $idpSet, "Unexpectedly got metadata for an alternate entity than that defined");
     }
 }
diff --git a/tests/lib/SimpleSAML/Metadata/test-metadata/source1/saml20-sp-remote.php b/tests/lib/SimpleSAML/Metadata/test-metadata/source1/saml20-sp-remote.php
new file mode 100644
index 000000000..379d1e45c
--- /dev/null
+++ b/tests/lib/SimpleSAML/Metadata/test-metadata/source1/saml20-sp-remote.php
@@ -0,0 +1,60 @@
+<?php
+
+$metadata['entityA'] = array(
+    'entityid' => 'entityA',
+    'name' =>
+        array(
+            'en' => 'entityA SP from source1',
+        ),
+    'metadata-set' => 'saml20-sp-remote',
+    'AssertionConsumerService' =>
+        array(
+            0 =>
+                array(
+                    'Binding' => 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST',
+                    'Location' => 'https://entityA.example.org/Shibboleth.sso/SAML2/POST',
+                    'index' => 1,
+                    'isDefault' => true,
+                ),
+        )
+);
+
+$metadata['entityInBoth'] = array(
+    'entityid' => 'entityInBoth',
+    'name' =>
+        array(
+            'en' => 'entityInBoth SP from source1',
+        ),
+    'metadata-set' => 'saml20-sp-remote',
+    'AssertionConsumerService' =>
+        array(
+            0 =>
+                array(
+                    'Binding' => 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST',
+                    'Location' => 'https://entityInBoth.example.org/Shibboleth.sso/SAML2/POST',
+                    'index' => 1,
+                    'isDefault' => true,
+                ),
+        )
+);
+
+$metadata['expiredInSrc1InSrc2'] = array(
+    'entityid' => 'expiredInSrc1InSrc2',
+    // This entity is expired in src1 but unexpired in src2
+    'expire' => 1,
+    'name' =>
+        array(
+            'en' => 'expiredInSrc1InSrc2 SP from source1',
+        ),
+    'metadata-set' => 'saml20-sp-remote',
+    'AssertionConsumerService' =>
+        array(
+            0 =>
+                array(
+                    'Binding' => 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST',
+                    'Location' => 'https://expiredInSrc1InSrc2.example.org/Shibboleth.sso/SAML2/POST',
+                    'index' => 1,
+                    'isDefault' => true,
+                ),
+        )
+);
\ No newline at end of file
diff --git a/tests/lib/SimpleSAML/Metadata/test-metadata/source2/saml20-sp-remote/entityB.serialized b/tests/lib/SimpleSAML/Metadata/test-metadata/source2/saml20-sp-remote/entityB.serialized
new file mode 100644
index 000000000..996e4a1f8
--- /dev/null
+++ b/tests/lib/SimpleSAML/Metadata/test-metadata/source2/saml20-sp-remote/entityB.serialized
@@ -0,0 +1 @@
+a:4:{s:8:"entityid";s:7:"entityB";s:4:"name";a:1:{s:2:"en";s:23:"entityB SP from source2";}s:12:"metadata-set";s:16:"saml20-sp-remote";s:24:"AssertionConsumerService";a:1:{i:0;a:4:{s:7:"Binding";s:46:"urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST";s:8:"Location";s:53:"https://entityB.example.org/Shibboleth.sso/SAML2/POST";s:5:"index";i:1;s:9:"isDefault";b:1;}}}
\ No newline at end of file
diff --git a/tests/lib/SimpleSAML/Metadata/test-metadata/source2/saml20-sp-remote/entityInBoth.serialized b/tests/lib/SimpleSAML/Metadata/test-metadata/source2/saml20-sp-remote/entityInBoth.serialized
new file mode 100644
index 000000000..fe0b1d351
--- /dev/null
+++ b/tests/lib/SimpleSAML/Metadata/test-metadata/source2/saml20-sp-remote/entityInBoth.serialized
@@ -0,0 +1 @@
+a:4:{s:8:"entityid";s:12:"entityInBoth";s:4:"name";a:1:{s:2:"en";s:28:"entityInBoth SP from source2";}s:12:"metadata-set";s:16:"saml20-sp-remote";s:24:"AssertionConsumerService";a:1:{i:0;a:4:{s:7:"Binding";s:46:"urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST";s:8:"Location";s:58:"https://entityInBoth.example.org/Shibboleth.sso/SAML2/POST";s:5:"index";i:1;s:9:"isDefault";b:1;}}}
\ No newline at end of file
diff --git a/tests/lib/SimpleSAML/Metadata/test-metadata/source2/saml20-sp-remote/expiredInSrc1InSrc2.serialized b/tests/lib/SimpleSAML/Metadata/test-metadata/source2/saml20-sp-remote/expiredInSrc1InSrc2.serialized
new file mode 100644
index 000000000..4c1379369
--- /dev/null
+++ b/tests/lib/SimpleSAML/Metadata/test-metadata/source2/saml20-sp-remote/expiredInSrc1InSrc2.serialized
@@ -0,0 +1 @@
+a:5:{s:8:"entityid";s:19:"expiredInSrc1InSrc2";s:6:"expire";i:3659688740;s:4:"name";a:1:{s:2:"en";s:35:"expiredInSrc1InSrc2 SP from source2";}s:12:"metadata-set";s:16:"saml20-sp-remote";s:24:"AssertionConsumerService";a:1:{i:0;a:4:{s:7:"Binding";s:46:"urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST";s:8:"Location";s:65:"https://expiredInSrc1InSrc2.example.org/Shibboleth.sso/SAML2/POST";s:5:"index";i:1;s:9:"isDefault";b:1;}}}
diff --git a/tests/modules/saml/lib/Auth/Source/Auth_Source_SP_Test.php b/tests/modules/saml/lib/Auth/Source/Auth_Source_SP_Test.php
index 53da3c2e9..6ead560d7 100644
--- a/tests/modules/saml/lib/Auth/Source/Auth_Source_SP_Test.php
+++ b/tests/modules/saml/lib/Auth/Source/Auth_Source_SP_Test.php
@@ -2,8 +2,14 @@
 
 namespace SimpleSAML\Test\Module\saml\Auth\Source;
 
+use InvalidArgumentException;
 use PHPUnit\Framework\TestCase;
+use SAML2\AuthnRequest;
 use \SimpleSAML\Configuration;
+use SimpleSAML\Module\saml\Error\NoAvailableIDP;
+use SimpleSAML\Module\saml\Error\NoSupportedIDP;
+use SimpleSAML\Test\Metadata\MetaDataStorageSourceTest;
+use SimpleSAML\Test\Utils\ClearStateTestCase;
 
 /**
  * Custom Exception to throw to terminate a TestCase.
@@ -68,7 +74,7 @@ class SPTester extends \SimpleSAML\Module\saml\Auth\Source\SP
 /**
  * Set of test cases for \SimpleSAML\Module\saml\Auth\Source\SP.
  */
-class SPTest extends TestCase
+class SPTest extends ClearStateTestCase
 {
 
     private $idpMetadata = null;
@@ -91,6 +97,7 @@ class SPTest extends TestCase
 
     protected function setUp()
     {
+        parent::setUp();
         $this->idpConfigArray = [
             'metadata-set'        => 'saml20-idp-remote',
             'entityid'            => 'https://engine.surfconext.nl/authentication/idp/metadata',
@@ -268,4 +275,154 @@ class SPTest extends TestCase
             $q[0]->value
         );
     }
+
+    /**
+     * Test specifying an IDPList where no metadata found for those idps is an error
+     */
+    public function testIdpListWithNoMatchingMetadata()
+    {
+        $this->expectException(NoSupportedIDP::class);
+        $state = [
+            'saml:IDPList' => ['noSuchIdp']
+        ];
+
+        $info = ['AuthId' => 'default-sp'];
+        $config = [];
+        $as = new SPTester($info, $config);
+        $as->authenticate($state);
+    }
+
+    /**
+     * Test specifying an IDPList where the list does not overlap with the Idp specified in SP config is an error
+     */
+    public function testIdpListWithExplicitIdpNotMatch()
+    {
+        $this->expectException(NoAvailableIDP::class);
+        $entityId = "https://example.com";
+        $xml = MetaDataStorageSourceTest::generateIdpMetadataXml($entityId);
+        $c = [
+            'metadata.sources' => [
+                ["type"=>"xml", "xml"=>$xml],
+            ],
+        ];
+        Configuration::loadFromArray($c, '', 'simplesaml');
+        $state = [
+            'saml:IDPList' => ['noSuchIdp', $entityId]
+        ];
+
+        $info = ['AuthId' => 'default-sp'];
+        $config = [
+            'idp' => 'https://engine.surfconext.nl/authentication/idp/metadata'
+        ];
+        $as = new SPTester($info, $config);
+        $as->authenticate($state);
+    }
+
+    /**
+     * Test that IDPList overlaps with the IDP specified in SP config results in AuthnRequest
+     */
+    public function testIdpListWithExplicitIdpMatch()
+    {
+        $entityId = "https://example.com";
+        $xml = MetaDataStorageSourceTest::generateIdpMetadataXml($entityId);
+        $c = [
+            'metadata.sources' => [
+                ["type"=>"xml", "xml"=>$xml],
+            ],
+        ];
+        Configuration::loadFromArray($c, '', 'simplesaml');
+        $state = [
+            'saml:IDPList' => ['noSuchIdp', $entityId]
+        ];
+
+        $info = ['AuthId' => 'default-sp'];
+        $config = [
+            'idp' => $entityId
+        ];
+        $as = new SPTester($info, $config);
+        try {
+            $as->authenticate($state);
+            $this->fail('Expected ExitTestException');
+        } catch (ExitTestException $e) {
+            $r = $e->getTestResult();
+            /** @var AuthnRequest $ar */
+            $ar = $r['ar'];
+            $xml = $ar->toSignedXML();
+            $q = \SAML2\Utils::xpQuery($xml, '/samlp:AuthnRequest/@Destination');
+            $this->assertEquals(
+                'https://saml.idp/sso/',
+                $q[0]->value
+            );
+        }
+    }
+
+    /**
+     * Test that IDPList with a single valid idp and no SP config idp results in AuthnRequest to that idp
+     */
+    public function testIdpListWithSingleMatch()
+    {
+        $entityId = "https://example.com";
+        $xml = MetaDataStorageSourceTest::generateIdpMetadataXml($entityId);
+        $c = [
+            'metadata.sources' => [
+                ["type"=>"xml", "xml"=>$xml],
+            ],
+        ];
+        Configuration::loadFromArray($c, '', 'simplesaml');
+        $state = [
+            'saml:IDPList' => ['noSuchIdp', $entityId]
+        ];
+
+        $info = ['AuthId' => 'default-sp'];
+        $config = [];
+        $as = new SPTester($info, $config);
+        try {
+            $as->authenticate($state);
+            $this->fail('Expected ExitTestException');
+        } catch (ExitTestException $e) {
+            $r = $e->getTestResult();
+            /** @var AuthnRequest $ar */
+            $ar = $r['ar'];
+            $xml = $ar->toSignedXML();
+            $q = \SAML2\Utils::xpQuery($xml, '/samlp:AuthnRequest/@Destination');
+            $this->assertEquals(
+                'https://saml.idp/sso/',
+                $q[0]->value
+            );
+        }
+    }
+
+    /**
+     * Test that IDPList with multiple valid idp and no SP config idp results in discovery redirect
+     */
+    public function testIdpListWithMultipleMatch()
+    {
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage('Invalid URL: smtp://invalidurl');
+        $entityId = "https://example.com";
+        $xml = MetaDataStorageSourceTest::generateIdpMetadataXml($entityId);
+        $entityId1 = "https://example1.com";
+        $xml1 = MetaDataStorageSourceTest::generateIdpMetadataXml($entityId1);
+        $c = [
+            'metadata.sources' => [
+                ["type"=>"xml", "xml"=>$xml],
+                ["type"=>"xml", "xml"=>$xml1],
+            ],
+        ];
+        Configuration::loadFromArray($c, '', 'simplesaml');
+        $state = [
+            'saml:IDPList' => ['noSuchIdp', $entityId, $entityId1]
+        ];
+
+        $info = ['AuthId' => 'default-sp'];
+        $config = [
+            // Use a url that is invalid for http redirects so redirect code throws an error
+            // otherwise it will call exit
+            'discoURL' => 'smtp://invalidurl'
+        ];
+        // Http redirect util library requires a request_uri to be set.
+        $_SERVER['REQUEST_URI'] = 'https://l.example.com/';
+        $as = new SPTester($info, $config);
+        $as->authenticate($state);
+    }
 }
-- 
GitLab