From 19b10688c3eedc8fa87545d5c2fd63308e090154 Mon Sep 17 00:00:00 2001 From: Kay Joosten Date: Thu, 7 May 2026 11:41:09 +0200 Subject: [PATCH 1/5] test: assert SAML ID format (prefix + 40 hex chars) --- .../Saml2/DefaultIdGeneratorTest.php | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 tests/unit/OpenConext/EngineBlock/Saml2/DefaultIdGeneratorTest.php diff --git a/tests/unit/OpenConext/EngineBlock/Saml2/DefaultIdGeneratorTest.php b/tests/unit/OpenConext/EngineBlock/Saml2/DefaultIdGeneratorTest.php new file mode 100644 index 000000000..aac6347f7 --- /dev/null +++ b/tests/unit/OpenConext/EngineBlock/Saml2/DefaultIdGeneratorTest.php @@ -0,0 +1,65 @@ +generator = new DefaultIdGenerator(); + } + + #[Test] + public function generateReturnsStringWithGivenPrefix(): void + { + $id = $this->generator->generate('EB', IdGenerator::ID_USAGE_OTHER); + + self::assertStringStartsWith('EB', $id); + } + + #[Test] + public function generateReturnsUniqueIds(): void + { + $id1 = $this->generator->generate(); + $id2 = $this->generator->generate(); + + self::assertNotSame($id1, $id2); + } + + #[Test] + public function generateDefaultsToEbPrefix(): void + { + $id = $this->generator->generate(); + + self::assertStringStartsWith('EB', $id); + } + + #[Test] + public function generateReturnsFortyHexCharsAfterPrefix(): void + { + $id = $this->generator->generate('EB', IdGenerator::ID_USAGE_OTHER); + + self::assertMatchesRegularExpression('/^EB[0-9a-f]{40}$/', $id); + } +} From f2ce0921c9da7c40b1af7549a1c1c22044d648d8 Mon Sep 17 00:00:00 2001 From: Kay Joosten Date: Thu, 7 May 2026 11:42:56 +0200 Subject: [PATCH 2/5] config: make compat saml2_id_generator a DI alias instead of a separate instance --- config/services/compat.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/services/compat.yml b/config/services/compat.yml index 44023e371..f90c0c6c0 100644 --- a/config/services/compat.yml +++ b/config/services/compat.yml @@ -54,7 +54,7 @@ services: engineblock.compat.saml2_id_generator: public: true - class: EngineBlock_Saml2_IdGenerator_Default + alias: OpenConext\EngineBlock\Saml2\IdGenerator engineblock.compat.attribute_release_policy_enforcer: public: false From 130ca9cf5c021508bd01f0102c3fde275b0469a8 Mon Sep 17 00:00:00 2001 From: Kay Joosten Date: Thu, 7 May 2026 11:45:21 +0200 Subject: [PATCH 3/5] refactor: replace legacy IdGenerator constant refs with new interface FQCN --- library/EngineBlock/Corto/Module/Service/SingleSignOn.php | 4 ++-- library/EngineBlock/Corto/ProxyServer.php | 6 +++--- library/EngineBlock/Saml2/AuthnRequestFactory.php | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/library/EngineBlock/Corto/Module/Service/SingleSignOn.php b/library/EngineBlock/Corto/Module/Service/SingleSignOn.php index 3fa431c70..854dfc26a 100644 --- a/library/EngineBlock/Corto/Module/Service/SingleSignOn.php +++ b/library/EngineBlock/Corto/Module/Service/SingleSignOn.php @@ -354,7 +354,7 @@ protected function _createUnsolicitedRequest() $relayState = !empty($_GET['RelayState']) ? $_GET['RelayState'] : null; $sspRequest = new AuthnRequest(); - $sspRequest->setId($this->_server->getNewId(EngineBlock_Saml2_IdGenerator::ID_USAGE_SAML2_REQUEST)); + $sspRequest->setId($this->_server->getNewId(\OpenConext\EngineBlock\Saml2\IdGenerator::ID_USAGE_SAML2_REQUEST)); $issuer = new Issuer(); $issuer->setValue($entityId); $sspRequest->setIssuer($issuer); @@ -389,7 +389,7 @@ protected function _createUnsolicitedRequest() protected function _createDebugRequest() { $sspRequest = new AuthnRequest(); - $sspRequest->setId($this->_server->getNewId(EngineBlock_Saml2_IdGenerator::ID_USAGE_SAML2_REQUEST)); + $sspRequest->setId($this->_server->getNewId(\OpenConext\EngineBlock\Saml2\IdGenerator::ID_USAGE_SAML2_REQUEST)); $issuer = new Issuer(); $issuer->setValue($this->_server->getUrl('spMetadataService')); $sspRequest->setIssuer($issuer); diff --git a/library/EngineBlock/Corto/ProxyServer.php b/library/EngineBlock/Corto/ProxyServer.php index ed645a191..f18d75cfa 100644 --- a/library/EngineBlock/Corto/ProxyServer.php +++ b/library/EngineBlock/Corto/ProxyServer.php @@ -800,7 +800,7 @@ public function createEnhancedResponse( // Create a new assertion by us. $newAssertion = new Assertion(); $newResponse->setAssertions(array($newAssertion)); - $newAssertion->setId($this->getNewId(EngineBlock_Saml2_IdGenerator::ID_USAGE_SAML2_ASSERTION)); + $newAssertion->setId($this->getNewId(\OpenConext\EngineBlock\Saml2\IdGenerator::ID_USAGE_SAML2_ASSERTION)); $newAssertion->setIssueInstant(time()); $newAssertion->setIssuer($newResponse->getIssuer()); @@ -892,7 +892,7 @@ protected function _createBaseResponse(EngineBlock_Saml2_AuthnRequestAnnotationD $response = new Response(); /** @var AuthnRequest $request */ $response->setRelayState($request->getRelayState()); - $response->setId($this->getNewId(EngineBlock_Saml2_IdGenerator::ID_USAGE_SAML2_RESPONSE)); + $response->setId($this->getNewId(\OpenConext\EngineBlock\Saml2\IdGenerator::ID_USAGE_SAML2_RESPONSE)); $response->setIssueInstant(time()); if (!$requestWasUnsolicited) { $response->setInResponseTo($request->getId()); @@ -1399,7 +1399,7 @@ public function timeStamp($deltaSeconds = 0) return $provider->timestamp($deltaSeconds); } - public function getNewId($usage = EngineBlock_Saml2_IdGenerator::ID_USAGE_OTHER) + public function getNewId($usage = \OpenConext\EngineBlock\Saml2\IdGenerator::ID_USAGE_OTHER) { $generator = EngineBlock_ApplicationSingleton::getInstance()->getDiContainer()->getSaml2IdGenerator(); return $generator->generate(self::ID_PREFIX, $usage); diff --git a/library/EngineBlock/Saml2/AuthnRequestFactory.php b/library/EngineBlock/Saml2/AuthnRequestFactory.php index b2311efa1..7901f0530 100644 --- a/library/EngineBlock/Saml2/AuthnRequestFactory.php +++ b/library/EngineBlock/Saml2/AuthnRequestFactory.php @@ -47,7 +47,7 @@ public static function createFromRequest( /** @var AuthnRequest $originalRequest */ $sspRequest = new AuthnRequest(); - $sspRequest->setId($server->getNewId(EngineBlock_Saml2_IdGenerator::ID_USAGE_SAML2_REQUEST)); + $sspRequest->setId($server->getNewId(\OpenConext\EngineBlock\Saml2\IdGenerator::ID_USAGE_SAML2_REQUEST)); $sspRequest->setIssueInstant(time()); $sspRequest->setDestination(isset($idpMetadata->singleSignOnServices[0]) ? $idpMetadata->singleSignOnServices[0]->location : null); $sspRequest->setForceAuthn($originalRequest->getForceAuthn()); From ecf65bf5d38ac19cf8d4c33f5a24d11e104a3caf Mon Sep 17 00:00:00 2001 From: Kay Joosten Date: Thu, 7 May 2026 15:47:29 +0200 Subject: [PATCH 4/5] feat: introduce OpenConext\EngineBlock\Saml2\IdGenerator interface and DefaultIdGenerator --- .../EngineBlock/Saml2/DefaultIdGenerator.php | 30 +++++++++++++++++++ .../EngineBlock/Saml2/IdGenerator.php | 30 +++++++++++++++++++ 2 files changed, 60 insertions(+) create mode 100644 src/OpenConext/EngineBlock/Saml2/DefaultIdGenerator.php create mode 100644 src/OpenConext/EngineBlock/Saml2/IdGenerator.php diff --git a/src/OpenConext/EngineBlock/Saml2/DefaultIdGenerator.php b/src/OpenConext/EngineBlock/Saml2/DefaultIdGenerator.php new file mode 100644 index 000000000..9a88454f2 --- /dev/null +++ b/src/OpenConext/EngineBlock/Saml2/DefaultIdGenerator.php @@ -0,0 +1,30 @@ + Date: Thu, 7 May 2026 15:47:33 +0200 Subject: [PATCH 5/5] feat: wire IdGenerator into Symfony DI and migrate MetadataRenderer from legacy interface --- config/services/services.yml | 8 +++++++- library/EngineBlock/Application/DiContainer.php | 2 +- src/OpenConext/EngineBlock/Xml/MetadataRenderer.php | 12 ++++++------ .../EngineBlock/Xml/MetadataRendererTest.php | 4 ++-- 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/config/services/services.yml b/config/services/services.yml index d7613ec3f..ba41aa6e7 100644 --- a/config/services/services.yml +++ b/config/services/services.yml @@ -167,11 +167,17 @@ services: - '@OpenConext\EngineBlock\Metadata\Factory\ValueObject\EngineBlockConfiguration' - '@OpenConext\EngineBlockBundle\Url\UrlProvider' + OpenConext\EngineBlock\Saml2\DefaultIdGenerator: ~ + + OpenConext\EngineBlock\Saml2\IdGenerator: + alias: OpenConext\EngineBlock\Saml2\DefaultIdGenerator + public: true + OpenConext\EngineBlock\Xml\MetadataRenderer: arguments: - '@OpenConext\EngineBlockBundle\Localization\LanguageSupportProvider' - '@twig' - - '@engineblock.compat.saml2_id_generator' + - '@OpenConext\EngineBlock\Saml2\IdGenerator' - '@OpenConext\EngineBlock\Metadata\X509\KeyPairFactory' - '@OpenConext\EngineBlock\Xml\DocumentSigner' - '@OpenConext\EngineBlock\Service\TimeProvider\TimeProvider' diff --git a/library/EngineBlock/Application/DiContainer.php b/library/EngineBlock/Application/DiContainer.php index 03d4a6f3e..27e881cc3 100644 --- a/library/EngineBlock/Application/DiContainer.php +++ b/library/EngineBlock/Application/DiContainer.php @@ -177,7 +177,7 @@ public function getTimeProvider() } /** - * @return EngineBlock_Saml2_IdGenerator + * @return \OpenConext\EngineBlock\Saml2\IdGenerator */ public function getSaml2IdGenerator() { diff --git a/src/OpenConext/EngineBlock/Xml/MetadataRenderer.php b/src/OpenConext/EngineBlock/Xml/MetadataRenderer.php index dc0c6a86c..949ccd677 100644 --- a/src/OpenConext/EngineBlock/Xml/MetadataRenderer.php +++ b/src/OpenConext/EngineBlock/Xml/MetadataRenderer.php @@ -18,7 +18,6 @@ namespace OpenConext\EngineBlock\Xml; -use EngineBlock_Saml2_IdGenerator; use InvalidArgumentException; use OpenConext\EngineBlock\Metadata\Factory\Collection\IdentityProviderEntityCollection; use OpenConext\EngineBlock\Metadata\Factory\Helper\IdentityProviderMetadataHelper; @@ -27,6 +26,7 @@ use OpenConext\EngineBlock\Metadata\Factory\ServiceProviderEntityInterface; use OpenConext\EngineBlock\Metadata\X509\KeyPairFactory; use OpenConext\EngineBlock\Metadata\X509\X509KeyPair; +use OpenConext\EngineBlock\Saml2\IdGenerator as SamlIdGenerator; use OpenConext\EngineBlock\Service\TimeProvider\TimeProvider; use OpenConext\EngineBlockBundle\Localization\LanguageSupportProvider; use Twig\Environment; @@ -49,7 +49,7 @@ class MetadataRenderer private $twig; /** - * @var EngineBlock_Saml2_IdGenerator + * @var SamlIdGenerator */ private $samlIdGenerator; @@ -83,7 +83,7 @@ class MetadataRenderer public function __construct( LanguageSupportProvider $languageSupportProvider, Environment $twig, - EngineBlock_Saml2_IdGenerator $samlIdGenerator, + SamlIdGenerator $samlIdGenerator, KeyPairFactory $keyPairFactory, DocumentSigner $documentSigner, TimeProvider $timeProvider, @@ -160,7 +160,7 @@ private function renderMetadataXmlServiceProvider(ServiceProviderEntityInterface } $params = [ - 'id' => $this->samlIdGenerator->generate(self::ID_PREFIX, EngineBlock_Saml2_IdGenerator::ID_USAGE_SAML2_METADATA), + 'id' => $this->samlIdGenerator->generate(self::ID_PREFIX, SamlIdGenerator::ID_USAGE_SAML2_METADATA), 'validUntil' => $this->getValidUntil(), 'metadata' => $metadata, 'locales' => $this->languageSupportProvider->getSupportedLanguages(), @@ -175,7 +175,7 @@ private function renderMetadataXmlIdentityProvider(IdentityProviderEntityInterfa $metadata = new IdentityProviderMetadataHelper($idp, $this->languageSupportProvider); $params = [ - 'id' => $this->samlIdGenerator->generate(self::ID_PREFIX, EngineBlock_Saml2_IdGenerator::ID_USAGE_SAML2_METADATA), + 'id' => $this->samlIdGenerator->generate(self::ID_PREFIX, SamlIdGenerator::ID_USAGE_SAML2_METADATA), 'validUntil' => $this->getValidUntil(), 'metadata' => $metadata, 'locales' => $this->languageSupportProvider->getSupportedLanguages(), @@ -192,7 +192,7 @@ private function renderMetadataXmlIdentityProviderCollection(IdentityProviderEnt } $params = [ - 'id' => $this->samlIdGenerator->generate(self::ID_PREFIX, EngineBlock_Saml2_IdGenerator::ID_USAGE_SAML2_METADATA), + 'id' => $this->samlIdGenerator->generate(self::ID_PREFIX, SamlIdGenerator::ID_USAGE_SAML2_METADATA), 'validUntil' => $this->getValidUntil(), 'metadataCollection' => $metadataCollection, 'locales' => $this->languageSupportProvider->getSupportedLanguages(), diff --git a/tests/unit/OpenConext/EngineBlock/Xml/MetadataRendererTest.php b/tests/unit/OpenConext/EngineBlock/Xml/MetadataRendererTest.php index b772a3cee..dcf530fe3 100644 --- a/tests/unit/OpenConext/EngineBlock/Xml/MetadataRendererTest.php +++ b/tests/unit/OpenConext/EngineBlock/Xml/MetadataRendererTest.php @@ -18,7 +18,7 @@ namespace OpenConext\EngineBlock\Xml; use DOMDocument; -use EngineBlock_Saml2_IdGenerator; +use OpenConext\EngineBlock\Saml2\IdGenerator as SamlIdGenerator; use Exception; use InvalidArgumentException; use Mockery as m; @@ -344,7 +344,7 @@ private function buildMetadataRenderer( $publicKey = new X509Certificate(openssl_x509_read(file_get_contents($basePath . '/tests/resources/key/engineblock.crt'))); $keyPair = new X509KeyPair($publicKey, $privateKey); - $samlIdGenerator = $this->createMock(EngineBlock_Saml2_IdGenerator::class); + $samlIdGenerator = $this->createMock(SamlIdGenerator::class); $samlIdGenerator->method('generate') ->willReturn('EB_metadata');