diff --git a/modules/saml/src/Controller/Metadata.php b/modules/saml/src/Controller/Metadata.php index ceae302e0b03426172cad2a5721ed915d155b9df..1498c82a86cd365b86d9ee21b4005450014e6c69 100644 --- a/modules/saml/src/Controller/Metadata.php +++ b/modules/saml/src/Controller/Metadata.php @@ -9,6 +9,7 @@ use SimpleSAML\Configuration; use SimpleSAML\Error; use SimpleSAML\HTTP\RunnableResponse; use SimpleSAML\Metadata as SSPMetadata; +use SimpleSAML\Metadata\MetaDataStorageHandler; use SimpleSAML\Module; use SimpleSAML\Module\saml\IdP\SAML2 as SAML2_IdP; use SimpleSAML\Utils; @@ -34,6 +35,7 @@ class Metadata /** @var \SimpleSAML\Utils\Auth */ protected Utils\Auth $authUtils; + protected MetadataStorageHandler $mdHandler; /** * Controller constructor. @@ -47,9 +49,9 @@ class Metadata ) { $this->config = $config; $this->authUtils = new Utils\Auth(); + $this->mdHandler = MetaDataStorageHandler::getMetadataHandler(); } - /** * Inject the \SimpleSAML\Utils\Auth dependency. * @@ -60,6 +62,13 @@ class Metadata $this->authUtils = $authUtils; } + /** + * Inject the \SimpleSAML\Metadata\MetadataStorageHandler dependency. + */ + public function setMetadataStorageHandler(MetadataStorageHandler $mdHandler): void + { + $this->mdHandler = $mdHandler; + } /** * This endpoint will offer the SAML 2.0 IdP metadata. @@ -78,15 +87,13 @@ class Metadata return new RunnableResponse([$this->authUtils, 'requireAdmin']); } - $metadata = SSPMetadata\MetaDataStorageHandler::getMetadataHandler(); - try { if ($request->query->has('idpentityid')) { $idpentityid = $request->query->get('idpentityid'); } else { - $idpentityid = $metadata->getMetaDataCurrentEntityID('saml20-idp-hosted'); + $idpentityid = $this->mdHandler->getMetaDataCurrentEntityID('saml20-idp-hosted'); } - $metaArray = SAML2_IdP::getHostedMetadata($idpentityid); + $metaArray = SAML2_IdP::getHostedMetadata($idpentityid, $this->mdHandler); $metaBuilder = new SSPMetadata\SAMLBuilder($idpentityid); $metaBuilder->addMetadataIdP20($metaArray); diff --git a/modules/saml/src/IdP/SAML2.php b/modules/saml/src/IdP/SAML2.php index 3fb76aa8d409142b56d7abedb6d3b20dc594bf18..54ff1a12efe51d92e4310df38d140410e4db45e8 100644 --- a/modules/saml/src/IdP/SAML2.php +++ b/modules/saml/src/IdP/SAML2.php @@ -747,15 +747,19 @@ class SAML2 * Retrieve the metadata of a hosted SAML 2 IdP. * * @param string $entityid The entity ID of the hosted SAML 2 IdP whose metadata we want. + * @param MetaDataStorageHandler $handler Optionally the metadata storage to use, + * if omitted the configured handler will be used. * * @return array * @throws \SimpleSAML\Error\CriticalConfigurationError * @throws \SimpleSAML\Error\Exception * @throws \SimpleSAML\Error\MetadataNotFound */ - public static function getHostedMetadata(string $entityid): array + public static function getHostedMetadata(string $entityid, MetaDataStorageHandler $handler = null): array { - $handler = MetaDataStorageHandler::getMetadataHandler(); + if ($handler === null) { + $handler = MetaDataStorageHandler::getMetadataHandler(); + } $config = $handler->getMetaDataConfig($entityid, 'saml20-idp-hosted'); // configure endpoints diff --git a/src/SimpleSAML/Auth/ProcessingChain.php b/src/SimpleSAML/Auth/ProcessingChain.php index 262e957442cc65cfa8c8c04b7a6cf828fa7bba20..c6a829cda6d0dcd46f71e7b9decd7c274c65eee8 100644 --- a/src/SimpleSAML/Auth/ProcessingChain.php +++ b/src/SimpleSAML/Auth/ProcessingChain.php @@ -126,7 +126,8 @@ class ProcessingChain if (!is_array($filter)) { throw new Exception( - "Invalid authentication processing filter configuration: One of the filters wasn't a string or an array." + "Invalid authentication processing filter configuration: " . + "One of the filters wasn't a string or an array." ); } diff --git a/tests/modules/saml/src/Controller/MetadataTest.php b/tests/modules/saml/src/Controller/MetadataTest.php index f507c9e55dc1172fd53d9f7cc63d1a48e5ba9a41..f106822b5e0f593c1470de0008a54c19000a2d59 100644 --- a/tests/modules/saml/src/Controller/MetadataTest.php +++ b/tests/modules/saml/src/Controller/MetadataTest.php @@ -6,7 +6,9 @@ namespace SimpleSAML\Test\Module\saml\Controller; use PHPUnit\Framework\TestCase; use SimpleSAML\Configuration; +use SimpleSAML\Error; use SimpleSAML\HTTP\RunnableResponse; +use SimpleSAML\Metadata\MetaDataStorageHandler; use SimpleSAML\Module\saml\Controller; use SimpleSAML\Session; use SimpleSAML\Utils; @@ -30,7 +32,7 @@ class MetadataTest extends TestCase /** @var \SimpleSAML\Utils\Auth */ protected Utils\Auth $authUtils; - + protected MetaDataStorageHandler $mdh; /** * Set up for each test. */ @@ -38,13 +40,69 @@ class MetadataTest extends TestCase { parent::setUp(); + $this->mdh = new class () extends MetaDataStorageHandler { + /** @var string */ + private const XMLSEC = '../vendor/simplesamlphp/xml-security/tests/resources'; + + /** @var string */ + public const CERT_KEY = self::XMLSEC . '/certificates/rsa-pem/selfsigned.simplesamlphp.org.key'; + + /** @var string */ + public const CERT_PUBLIC = self::XMLSEC . '/certificates/rsa-pem/selfsigned.simplesamlphp.org.crt'; + + private array $idps; + + public function __construct() + { + $this->idps = [ + 'urn:example:simplesaml:idp' => [ + 'name' => 'SimpleSAMLphp Hosted IDP', + 'descr' => 'The local IDP', + 'OrganizationDisplayName' => ['en' => 'My IDP', 'nl' => 'Mijn IDP'], + 'certificate' => self::CERT_PUBLIC, + 'privatekey' => self::CERT_KEY, + + ], + 'urn:example:simplesaml:another:idp' => [ + 'name' => 'SimpleSAMLphp Hosted Another IDP', + 'descr' => 'Different IDP', + 'OrganizationDisplayName' => ['en' => 'Other IDP', 'nl' => 'Andere IDP'], + 'certificate' => self::CERT_PUBLIC, + 'privatekey' => self::CERT_KEY, + + ], + ]; + } + + public function getMetaData(?string $entityId, string $set): array + { + if (isset($this->idps[$entityId]) && $set === 'saml20-idp-hosted') { + return $this->idps[$entityId]; + } + + throw new Error\MetadataNotFound($entityId ?? ''); + } + + public function getList(string $set = 'saml20-idp-remote', bool $showExpired = false): array + { + if ($set === 'saml20-idp-hosted') { + return $idps; + } + return []; + } + + public function getMetaDataCurrentEntityID(string $set, string $type = 'entityid'): string + { + return 'urn:example:simplesaml:another:idp'; + } + }; $this->session = Session::getSessionFromRequest(); $this->config = Configuration::loadFromArray( [ 'module.enable' => ['saml' => true], 'enable.saml20-idp' => true, - 'admin.protectmetadata' => true, + 'admin.protectmetadata' => false, ], '[ARRAY]', 'simplesaml' @@ -70,6 +128,8 @@ class MetadataTest extends TestCase // stub } }; + + $_SERVER['REQUEST_URI'] = '/dummy'; } @@ -78,39 +138,43 @@ class MetadataTest extends TestCase * and admin.protectmetadata set to true or false is handled properly * * @dataProvider provideMetadataAccess - * @param bool $protected - * @param bool $authenticated - * @return void */ public function testMetadataAccess(bool $authenticated, bool $protected): void { + $config = Configuration::loadFromArray( + [ + 'module.enable' => ['saml' => true], + 'enable.saml20-idp' => true, + 'admin.protectmetadata' => $protected, + ], + '[ARRAY]', + 'simplesaml' + ); + Configuration::setPreLoadedConfig($config, 'config.php'); + $request = Request::create( '/idp/metadata', 'GET', ); - $c = new Controller\Metadata($this->config, $this->session); + $c = new Controller\Metadata($config, $this->session); + $c->setMetadataStorageHandler($this->mdh); - if ($authenticated === true || $protected === false) { + if ($authenticated === true) { // Bypass authentication - mock being authenticated $c->setAuthUtils($this->authUtils); } $result = $c->metadata($request); - if ($authenticated !== false && $protected !== true) { - // ($authenticated === true) or ($protected === false) - // Should lead to a Response - $this->assertInstanceOf(Response::class, $result); - } else { + if ($protected && !$authenticated) { $this->assertInstanceOf(RunnableResponse::class, $result); + $this->assertEquals("requireAdmin", $result->getCallable()[1]); + } else { + $this->assertInstanceOf(Response::class, $result); } } - - /** - * @return array - */ public function provideMetadataAccess(): array { return [ @@ -121,4 +185,117 @@ class MetadataTest extends TestCase [true, true], ]; } + + /** + * Test that saml20-idp setting disabled disables access + */ + public function testDisabledSAML20IDPReturnsNoAccess(): void + { + $config = Configuration::loadFromArray( + [ + 'module.enable' => ['saml' => true], + 'enable.saml20-idp' => false, + ], + '[ARRAY]', + 'simplesaml' + ); + Configuration::setPreLoadedConfig($config, 'config.php'); + + $request = Request::create( + '/idp/metadata', + 'GET', + ); + + $c = new Controller\Metadata($config, $this->session); + + $this->expectException(\SimpleSAML\Error\Error::class); + $this->expectExceptionMessage('NOACCESS'); + $result = $c->metadata($request); + } + + /** + * Test that requesting a non-existing entityID throws an exception + */ + public function testMetadataUnknownEntityThrowsError(): void + { + $request = Request::create( + '/idp/metadata?idpentityid=https://example.org/notexist', + 'GET', + ); + + $c = new Controller\Metadata($this->config, $this->session); + $c->setMetadataStorageHandler($this->mdh); + + $this->expectException(\SimpleSAML\Error\Error::class); + $this->expectExceptionMessage('METADATA'); + $result = $c->metadata($request); + } + + /** + * Basic smoke test of generated metadata + */ + public function testMetadataYieldsContent(): void + { + $request = Request::create( + '/idp/metadata?idpentityid=urn:example:simplesaml:idp', + 'GET', + ); + + $c = new Controller\Metadata($this->config, $this->session); + $c->setMetadataStorageHandler($this->mdh); + + $result = $c->metadata($request); + $this->assertEquals('application/samlmetadata+xml', $result->headers->get('Content-Type')); + $this->assertEquals('attachment; filename="idp-metadata.xml"', $result->headers->get('Content-Disposition')); + $content = $result->getContent(); + + $expect = 'entityID="urn:example:simplesaml:idp"'; + $this->assertStringContainsString($expect, $content); + + $expect = '<md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location='; + $this->assertStringContainsString($expect, $content); + } + + /** + * Test not specifying explict entityID falls back to a default + */ + public function testMetadataDefaultIdPYieldsContent(): void + { + $request = Request::create( + '/idp/metadata', + 'GET', + ); + + $c = new Controller\Metadata($this->config, $this->session); + $c->setMetadataStorageHandler($this->mdh); + + $result = $c->metadata($request); + $this->assertEquals('application/samlmetadata+xml', $result->headers->get('Content-Type')); + $this->assertEquals('attachment; filename="idp-metadata.xml"', $result->headers->get('Content-Disposition')); + $content = $result->getContent(); + + $expect = 'entityID="urn:example:simplesaml:another:idp"'; + $this->assertStringContainsString($expect, $content); + + $expect = '<md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location='; + $this->assertStringContainsString($expect, $content); + } + + /** + * Check if caching headers are set + */ + public function testMetadataCachingHeaders(): void + { + $request = Request::create( + '/idp/metadata', + 'GET', + ); + + $c = new Controller\Metadata($this->config, $this->session); + $c->setMetadataStorageHandler($this->mdh); + + $result = $c->metadata($request); + $this->assertTrue($result->headers->has('ETag')); + $this->assertEquals('public', $result->headers->get('Cache-Control')); + } } diff --git a/tests/modules/saml/src/Controller/ServiceProviderTest.php b/tests/modules/saml/src/Controller/ServiceProviderTest.php index 5636553c5dbd8ca1905a08415d69fc9fdca40ecd..0ca01986fd1abbfd7f76a6f2967abaae498b8bc7 100644 --- a/tests/modules/saml/src/Controller/ServiceProviderTest.php +++ b/tests/modules/saml/src/Controller/ServiceProviderTest.php @@ -34,7 +34,6 @@ class ServiceProviderTest extends TestCase /** @var \SimpleSAML\Utils\Auth */ protected Utils\Auth $authUtils; - /** * Set up for each test. */ @@ -46,7 +45,7 @@ class ServiceProviderTest extends TestCase $this->config = Configuration::loadFromArray( [ 'module.enable' => ['saml' => true], - 'admin.protectmetadata' => true, + 'admin.protectmetadata' => false, ], '[ARRAY]', 'simplesaml' @@ -75,6 +74,8 @@ class ServiceProviderTest extends TestCase // stub } }; + + $_SERVER['REQUEST_URI'] = '/dummy'; } @@ -398,34 +399,41 @@ XML; * and admin.protectmetadata set to true or false is handled properly * * @dataProvider provideMetadataAccess - * @param bool $protected - * @param bool $authenticated - * @return void */ public function testMetadataAccess(bool $authenticated, bool $protected): void { - $c = new Controller\ServiceProvider($this->config, $this->session); + $config = Configuration::loadFromArray( + [ + 'module.enable' => ['saml' => true], + 'admin.protectmetadata' => $protected, + ], + '[ARRAY]', + 'simplesaml' + ); + Configuration::setPreLoadedConfig($config, 'config.php'); + + $request = Request::create( + '/sp/metadata/phpunit', + 'GET', + ); + + $c = new Controller\ServiceProvider($config, $this->session); if ($authenticated === true || $protected === false) { // Bypass authentication - mock being authenticated $c->setAuthUtils($this->authUtils); } - $result = $c->metadata('phpunit'); + $result = $c->metadata($request, 'phpunit'); - if ($authenticated !== false && $protected !== true) { - // ($authenticated === true) or ($protected === false) - // Should lead to a Response - $this->assertInstanceOf(Response::class, $result); - } else { + if ($protected && !$authenticated) { $this->assertInstanceOf(RunnableResponse::class, $result); + $this->assertEquals("requireAdmin", $result->getCallable()[1]); + } else { + $this->assertInstanceOf(Response::class, $result); } } - - /** - * @return array - */ public function provideMetadataAccess(): array { return [ @@ -436,4 +444,58 @@ XML; [true, true], ]; } + + /** + * Test that requesting a non-existing authsource yields an error + */ + public function testMetadataUnknownEntityThrowsError(): void + { + $request = Request::create( + '/sp/metadata/phpnonunit', + 'GET', + ); + + $c = new Controller\ServiceProvider($this->config, $this->session); + + $this->expectException(\SimpleSAML\Error\Error::class); + $this->expectExceptionMessage("Error with authentication source 'phpnonunit': Could not find authentication source."); + $result = $c->metadata($request, 'phpnonunit'); + } + + /** + * Basic smoke test of generated metadata + */ + public function testMetadataYieldsContent(): void + { + $request = Request::create( + '/sp/metadata/phpunit', + 'GET', + ); + + $c = new Controller\ServiceProvider($this->config, $this->session); + + $result = $c->metadata($request, 'phpunit'); + $this->assertEquals('application/samlmetadata+xml', $result->headers->get('Content-Type')); + $this->assertEquals('attachment; filename="phpunit.xml"', $result->headers->get('Content-Disposition')); + $content = $result->getContent(); + $expect = 'entityID="urn:x-simplesamlphp:example-sp"'; + $this->assertStringContainsString($expect, $content); + } + + /** + * Check if caching headers are set + */ + public function testMetadataCachingHeaders(): void + { + $request = Request::create( + '/sp/metadata/phpunit', + 'GET', + ); + + $c = new Controller\ServiceProvider($this->config, $this->session); + + $result = $c->metadata($request, 'phpunit'); + $this->assertTrue($result->headers->has('ETag')); + $this->assertEquals('public', $result->headers->get('Cache-Control')); + } }