Add rollover metadata containing all available keys (#1899)

tvdijen · kayjoosten · web-flow · commit 5c3ca2a8cf5c · 2026-03-27T09:49:20.000+01:00
* Add rollover metadata containing all available keys

* Fix CI

* Add tests for rollover metadata key support

- Fix broken unit tests in decorator classes after array change
- Add buildAll() unit test to KeyPairFactoryTest
- Add Behat scenarios verifying all keys appear in default metadata
- Update FrontPage test to expect rollover key links
- Add CI and dev env parameters with rollover key for testing
- Remove unused X509Certificate import from EngineBlockServiceProvider

---------

Co-authored-by: Kay Joosten &lt;kay.joosten@dawn.tech&gt;
diff --git a/config/packages/ci/parameters.yml b/config/packages/ci/parameters.yml
@@ -0,0 +1,8 @@
+parameters:
+    encryption_keys:
+        default:
+            publicFile: /config/engine/engineblock.crt
+            privateFile: /config/engine/engineblock.pem
+        rollover:
+            publicFile: '%kernel.project_dir%/src/OpenConext/EngineBlockFunctionalTestingBundle/Resources/keys/rolled-over.crt'
+            privateFile: '%kernel.project_dir%/src/OpenConext/EngineBlockFunctionalTestingBundle/Resources/keys/rolled-over.key'
diff --git a/config/packages/dev/parameters.yml b/config/packages/dev/parameters.yml
@@ -0,0 +1,8 @@
+parameters:
+    encryption_keys:
+        default:
+            publicFile: /config/engine/engineblock.crt
+            privateFile: /config/engine/engineblock.pem
+        rollover:
+            publicFile: '%kernel.project_dir%/src/OpenConext/EngineBlockFunctionalTestingBundle/Resources/keys/rolled-over.crt'
+            privateFile: '%kernel.project_dir%/src/OpenConext/EngineBlockFunctionalTestingBundle/Resources/keys/rolled-over.key'
diff --git a/config/packages/parameters.yml.dist b/config/packages/parameters.yml.dist
@@ -36,6 +36,9 @@ parameters:
     ## The Signing / Encryption keys used for the SAML2 authentication and metadata
     ## When EngineBlock signs responses (when it acts as an Idp)
     ## or requests (when it acts as an SP) it uses these X.509 certs.
+    ## During a key rollover, add the new key as 'rollover'. The default metadata will then
+    ## contain both keys, allowing IdPs to accept responses signed with either key.
+    ## Once all IdPs have updated their metadata, remove the old key or swap default/rollover.
     encryption_keys:
         default:
             publicFile: /config/engine/engineblock.crt
diff --git a/src/OpenConext/EngineBlock/Metadata/Factory/Decorator/EngineBlockIdentityProvider.php b/src/OpenConext/EngineBlock/Metadata/Factory/Decorator/EngineBlockIdentityProvider.php
@@ -30,9 +30,10 @@
 class EngineBlockIdentityProvider extends AbstractIdentityProvider
 {
     /**
-     * @var X509KeyPair
+     * @var array<X509KeyPair>
      */
-    private $keyPair;
+    private array $keyPairs;
+
     /**
      * @var UrlProvider
      */
@@ -46,20 +47,23 @@ class EngineBlockIdentityProvider extends AbstractIdentityProvider
     public function __construct(
         IdentityProviderEntityInterface $entity,
         ?string $keyId,
-        X509KeyPair $keyPair,
+        array $keyPairs,
         UrlProvider $urlProvider
     ) {
         parent::__construct($entity);
         $this->keyId = $keyId;
-        $this->keyPair = $keyPair;
+        $this->keyPairs = $keyPairs;
         $this->urlProvider = $urlProvider;
     }
 
     public function getCertificates(): array
     {
-        return [
-            $this->keyPair->getCertificate(),
-        ];
+        $certificates = [];
+        foreach ($this->keyPairs as $keyPair) {
+            $certificates[] = $keyPair->getCertificate();
+        }
+
+        return $certificates;
     }
 
     public function getSupportedNameIdFormats(): array
diff --git a/src/OpenConext/EngineBlock/Metadata/Factory/Decorator/EngineBlockServiceProvider.php b/src/OpenConext/EngineBlock/Metadata/Factory/Decorator/EngineBlockServiceProvider.php
@@ -32,35 +32,42 @@
 class EngineBlockServiceProvider extends AbstractServiceProvider
 {
     /**
-     * @var X509KeyPair
+     * @var array<X509KeyPair>
      */
-    private $keyPair;
+    private array $keyPairs;
+
     /**
      * @var AttributesMetadata
      */
     private $attributes;
+
     /**
      * @var UrlProvider
      */
     private $urlProvider;
 
     public function __construct(
         ServiceProviderEntityInterface $entity,
-        X509KeyPair $keyPair,
+        array $keyPairs,
         AttributesMetadata $attributes,
         UrlProvider $urlProvider
     ) {
         parent::__construct($entity);
 
-        $this->keyPair = $keyPair;
+        $this->keyPairs = $keyPairs;
         $this->attributes = $attributes;
         $this->urlProvider = $urlProvider;
     }
 
 
     public function getCertificates(): array
     {
-        return [$this->keyPair->getCertificate()];
+        $certificates = [];
+        foreach ($this->keyPairs as $keyPair) {
+            $certificates[] = $keyPair->getCertificate();
+        }
+
+        return $certificates;
     }
 
     /**
diff --git a/src/OpenConext/EngineBlock/Metadata/Factory/Factory/IdentityProviderFactory.php b/src/OpenConext/EngineBlock/Metadata/Factory/Factory/IdentityProviderFactory.php
@@ -100,7 +100,9 @@ private function buildEngineBlockEntityFromEntity(IdentityProvider $entity, ?str
                 $this->engineBlockConfiguration
             ),
             $keyId,
-            $this->keyPairFactory->buildFromIdentifier($keyId),
+            ($keyId === KeyPairFactory::DEFAULT_KEY_PAIR_IDENTIFIER || $keyId === null)
+                ? $this->keyPairFactory->buildAll()
+                : [$this->keyPairFactory->buildFromIdentifier($keyId)],
             $this->urlProvider
         );
     }
diff --git a/src/OpenConext/EngineBlock/Metadata/Factory/Factory/ServiceProviderFactory.php b/src/OpenConext/EngineBlock/Metadata/Factory/Factory/ServiceProviderFactory.php
@@ -53,6 +53,7 @@ class ServiceProviderFactory
      * @var EngineBlockConfiguration
      */
     private $engineBlockConfiguration;
+
     /**
      * @var UrlProvider
      */
@@ -95,7 +96,9 @@ public function createEngineBlockEntityFrom(string $keyId): ServiceProviderEntit
                 new ServiceProviderEntity($entity),
                 $this->engineBlockConfiguration
             ),
-            $this->keyPairFactory->buildFromIdentifier($keyId),
+            ($keyId === KeyPairFactory::DEFAULT_KEY_PAIR_IDENTIFIER || $keyId === null)
+                ? $this->keyPairFactory->buildAll()
+                : [$this->keyPairFactory->buildFromIdentifier($keyId)],
             $this->attributes,
             $this->urlProvider
         );
diff --git a/src/OpenConext/EngineBlock/Metadata/X509/KeyPairFactory.php b/src/OpenConext/EngineBlock/Metadata/X509/KeyPairFactory.php
@@ -26,7 +26,7 @@ class KeyPairFactory
 {
     const DEFAULT_KEY_PAIR_IDENTIFIER = 'default';
 
-    private $keyPairConfiguration = [];
+    private array $keyPairConfiguration = [];
 
     /**
      * @param array $keyPairConfiguration
@@ -42,7 +42,7 @@ public function __construct(array $keyPairConfiguration)
      *
      * @throws RuntimeException
      */
-    public function buildFromIdentifier(?string $identifier) : X509KeyPair
+    public function buildFromIdentifier(?string $identifier): X509KeyPair
     {
         if ($identifier === null) {
             $identifier = self::DEFAULT_KEY_PAIR_IDENTIFIER;
@@ -57,4 +57,20 @@ public function buildFromIdentifier(?string $identifier) : X509KeyPair
         }
         throw new UnknownKeyIdException($identifier);
     }
+
+    /**
+     * @return array<X509KeyPair>
+     *
+     * @throws RuntimeException
+     */
+    public function buildAll(): array
+    {
+        $pairs = [];
+
+        foreach (array_keys($this->keyPairConfiguration) as $keyId) {
+            $pairs[] = $this->buildFromIdentifier((string)$keyId);
+        }
+
+        return $pairs;
+    }
 }
diff --git a/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/FrontPage.feature b/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/FrontPage.feature
@@ -10,20 +10,25 @@ Feature:
 
   Scenario: The user can see all available metadata links
     When I go to Engineblock URL "/"
-    Then I should see 8 links on the front page
+    Then I should see 12 links on the front page
      And I should see text matching "The Public SAML Signing certificate of the OpenConext IdP"
      And I should see URL "/authentication/idp/certificate"
      # The default key should not get a separate URL next to the key-less url
      And I should not see URL "/authentication/idp/certificate/key:default"
+     # The rollover key should get its own URL
+     And I should see URL "/authentication/idp/certificate/key:rollover"
      And I should see text matching "The Public SAML metadata \(the entity descriptor\) of the OpenConext IdP Proxy"
      And I should see URL "/authentication/idp/metadata"
      And I should not see URL "/authentication/idp/metadata/key:default"
+     And I should see URL "/authentication/idp/metadata/key:rollover"
      And I should see text matching "The Public SAML metadata \(the entities descriptor\) for all the OpenConext IdPs"
      And I should see URL "/authentication/proxy/idps-metadata"
      And I should not see URL "/authentication/proxy/idps-metadata/key:default"
+     And I should see URL "/authentication/proxy/idps-metadata/key:rollover"
      And I should see text matching "The Public SAML metadata \(the entities descriptor\) of the OpenConext IdPs which"
      And I should see URL "/authentication/proxy/idps-metadata?sp-entity-id=urn:example.org"
      And I should not see URL "/authentication/proxy/idps-metadata/key:default?sp-entity-id=urn:example.org"
+     And I should see URL "/authentication/proxy/idps-metadata/key:rollover?sp-entity-id=urn:example.org"
      # The test debug link is present on the page
      And I should see text matching "Test authentication with an identity provider."
      And I should see URL "/authentication/sp/debug"
diff --git a/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Metadata.feature b/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Metadata.feature
@@ -220,6 +220,33 @@ Feature:
       # Verify the used signing key is EB key
       And the response should match xpath '//ds:Signature//ds:X509Certificate[starts-with(.,"MIIDuDCCAqCgAwIBAgIJAPdqJ9JQKN6vMA0GCSqGSIb3DQEBBQUAMEYxDzANBgNVBAMT")]'
 
+  Scenario: The default SP metadata contains all configured keys (rollover support)
+    When I go to Engineblock URL "/authentication/sp/metadata"
+    # Verify the default (current) signing key is present
+    Then the response should match xpath '//md:KeyDescriptor[@use="signing"]//ds:X509Certificate[starts-with(.,"MIIDuDCCAqCgAwIBAgIJAPdqJ9JQKN6vMA0GCSqGSIb3DQEBBQUAMEYxDzANBgNVBAMT")]'
+    # Verify the rollover signing key is also present
+    And the response should match xpath '//md:KeyDescriptor[@use="signing"]//ds:X509Certificate[starts-with(.,"MIIDhTCCAm2gAwIBAgIJALJlbT5u9cXzMA0GCSqGSIb3DQEBBQUAMFkxCzAJ")]'
+    # Verify the metadata document is signed with the default key
+    And the response should match xpath '//ds:Signature//ds:X509Certificate[starts-with(.,"MIIDuDCCAqCgAwIBAgIJAPdqJ9JQKN6vMA0GCSqGSIb3DQEBBQUAMEYxDzANBgNVBAMT")]'
+
+  Scenario: The default IdP metadata contains all configured keys (rollover support)
+    When I go to Engineblock URL "/authentication/idp/metadata"
+    # Verify the default (current) signing key is present
+    Then the response should match xpath '//md:KeyDescriptor[@use="signing"]//ds:X509Certificate[starts-with(.,"MIIDuDCCAqCgAwIBAgIJAPdqJ9JQKN6vMA0GCSqGSIb3DQEBBQUAMEYxDzANBgNVBAMT")]'
+    # Verify the rollover signing key is also present
+    And the response should match xpath '//md:KeyDescriptor[@use="signing"]//ds:X509Certificate[starts-with(.,"MIIDhTCCAm2gAwIBAgIJALJlbT5u9cXzMA0GCSqGSIb3DQEBBQUAMFkxCzAJ")]'
+    # Verify the metadata document is signed with the default key
+    And the response should match xpath '//ds:Signature//ds:X509Certificate[starts-with(.,"MIIDuDCCAqCgAwIBAgIJAPdqJ9JQKN6vMA0GCSqGSIb3DQEBBQUAMEYxDzANBgNVBAMT")]'
+
+  Scenario: Requesting metadata with a specific key id returns only that key
+    When I go to Engineblock URL "/authentication/sp/metadata/key:rollover"
+    # Verify only the rollover key is present
+    Then the response should match xpath '//md:KeyDescriptor[@use="signing"]//ds:X509Certificate[starts-with(.,"MIIDhTCCAm2gAwIBAgIJALJlbT5u9cXzMA0GCSqGSIb3DQEBBQUAMFkxCzAJ")]'
+    # Verify the default key is not present
+    And the response should not match xpath '//md:KeyDescriptor[@use="signing"]//ds:X509Certificate[starts-with(.,"MIIDuDCCAqCgAwIBAgIJAPdqJ9JQKN6vMA0GCSqGSIb3DQEBBQUAMEYxDzANBgNVBAMT")]'
+    # Verify the metadata document is signed with the rollover key
+    And the response should match xpath '//ds:Signature//ds:X509Certificate[starts-with(.,"MIIDhTCCAm2gAwIBAgIJALJlbT5u9cXzMA0GCSqGSIb3DQEBBQUAMFkxCzAJ")]'
+
   Scenario: A user can request the EngineBlock SP public certificate
     When I go to Engineblock URL "/authentication/sp/certificate"
     Then the response should contain '-----BEGIN CERTIFICATE-----'
diff --git a/tests/unit/OpenConext/EngineBlock/Metadata/Factory/Decorator/EngineBlockServiceProvider.php b/tests/unit/OpenConext/EngineBlock/Metadata/Factory/Decorator/EngineBlockServiceProvider.php
@@ -74,7 +74,7 @@ function (...$parameters) use ($matcher) {
         $attributesMock->method('getRequestedAttributes')
             ->willReturn($attributes);
 
-        $decorator = new EngineBlockServiceProvider($adapter, $keyPairMock, $attributesMock, $this->urlProvider);
+        $decorator = new EngineBlockServiceProvider($adapter, [$keyPairMock], $attributesMock, $this->urlProvider);
 
         $supportedNameIdFormats = [
             Constants::NAMEID_PERSISTENT,
diff --git a/tests/unit/OpenConext/EngineBlock/Metadata/Factory/Decorator/EngineblockIdentityProvider.php b/tests/unit/OpenConext/EngineBlock/Metadata/Factory/Decorator/EngineblockIdentityProvider.php
@@ -85,7 +85,7 @@ public function test_methods()
                 }
             });
 
-        $decorator = new EngineBlockIdentityProvider($this->adapter, 'default', $this->keyPairMock, $this->urlProvider);
+        $decorator = new EngineBlockIdentityProvider($this->adapter, 'default', [$this->keyPairMock], $this->urlProvider);
 
         $supportedNameIdFormats = [
             Constants::NAMEID_PERSISTENT,
@@ -110,7 +110,7 @@ public function test_override_slo_service_if_child_slo_service_set()
             ->with('authentication_logout', false, null, null)
             ->willReturn('sloLocation');
 
-        $decorator = new EngineBlockIdentityProvider($this->adapter, 'default', $this->keyPairMock, $this->urlProvider);
+        $decorator = new EngineBlockIdentityProvider($this->adapter, 'default', [$this->keyPairMock], $this->urlProvider);
 
         $this->assertEquals($decorator->getSingleLogoutService(), new Service('sloLocation', Constants::BINDING_HTTP_REDIRECT));
     }
@@ -121,7 +121,7 @@ public function test_return_null_for_slo_service_if_child_has_no_slo_service_set
             'singleLogoutService' => null,
         ]);
 
-        $decorator = new EngineBlockIdentityProvider($this->adapter, 'default', $this->keyPairMock, $this->urlProvider);
+        $decorator = new EngineBlockIdentityProvider($this->adapter, 'default', [$this->keyPairMock], $this->urlProvider);
 
         $this->assertEquals($decorator->getSingleLogoutService(), null);
     }
@@ -145,7 +145,7 @@ function (...$parameters) use ($matcher) {
                 }  // we would expect the fourth paremeter to be null and not 'entity-id' becasue we are EB
             );
 
-        $decorator = new EngineBlockIdentityProvider($this->adapter, 'default', $this->keyPairMock, $this->urlProvider);
+        $decorator = new EngineBlockIdentityProvider($this->adapter, 'default', [$this->keyPairMock], $this->urlProvider);
 
         $this->assertEquals([new Service('ssoLocation', Constants::BINDING_HTTP_REDIRECT)], $decorator->getSingleSignOnServices());
     }
diff --git a/tests/unit/OpenConext/EngineBlock/Metadata/X509/KeyPairFactoryTest.php b/tests/unit/OpenConext/EngineBlock/Metadata/X509/KeyPairFactoryTest.php
@@ -54,6 +54,16 @@ public function test_builds_key_pair_from_specified_identifier()
         $this->assertEquals('file://' . __DIR__ . '/test.pem.key', $defaultKeyPair->getPrivateKey()->getFilePath());
     }
 
+    public function test_build_all_returns_all_configured_key_pairs()
+    {
+        $keyPairs = $this->factory->buildAll();
+
+        $this->assertCount(2, $keyPairs);
+        $this->assertContainsOnlyInstancesOf(X509KeyPair::class, $keyPairs);
+        $this->assertEquals(file_get_contents('file://' . __DIR__ . '/test.pem.crt'), $keyPairs[0]->getCertificate()->toPem());
+        $this->assertEquals(file_get_contents('file://' . __DIR__ . '/test2.pem.crt'), $keyPairs[1]->getCertificate()->toPem());
+    }
+
     public function test_it_raises_exception_when_requesting_empty_key_pair_identifier()
     {
         $this->expectException(InvalidArgumentException::class);
