Make metadata expiration time configurable (#1860)

* Make metadata expiration time configurable

* Add sanity checks & test coverage on parameter & cleanup & docs

---------

Co-authored-by: Johan Kromhout <johan.kromhout@dawn.tech>

Author: AramMutluWork

CHANGELOG.md

@@ -22,6 +22,8 @@ Maintenance:
 * `symfony/monolog-bundle` upgraded to ^4.0; review your monolog configuration if you have customised it outside of the defaults.
 
 Changes:
+* The metadata expiration time (`validUntil` attribute) is now configurable via the `metadata_expiration_time` parameter in `parameters.yml`.
+  * Action required: Add `metadata_expiration_time` to `parameters.yaml`, suggested value: `86400`. This is the old behaviour in which metadata is cached for 24 hours.
 * The `consent.deleted_at` should be not nullable, and have a default value of `0000-00-00 00:00:00`.
   * Because `deleted_at` is part of the PK, no migration is provided. The database engine should not allow this to be null in the first place, so it is probably not nullable already on your db.
   * The `0000-00-00 00:00:00` is added for clarity/consistency, as this is probably the default behaviour of your database already.

config/packages/parameters.yml.dist

@@ -55,6 +55,8 @@ parameters:
     ## Add RequestedAttributes to the AttributeConsumingService of the SP Proxy metadata of Engineblock, default is all
     ## Options are 'all' (optional and required attributes), 'required' (only required attributes) or 'none'
     metadata_add_requested_attributes: all
+    ## The number of seconds a Metadata document is deemed valid (default 24h). Must be a positive integer.
+    metadata_expiration_time: 86400
 
     ##########################################################################################
     ## PHP SETTINGS

config/services/services.yml

@@ -157,6 +157,7 @@ services:
             - '@OpenConext\EngineBlock\Xml\DocumentSigner'
             - '@OpenConext\EngineBlock\Service\TimeProvider\TimeProvider'
             - '%metadata_add_requested_attributes%'
+            - "%metadata_expiration_time%"
 
     OpenConext\EngineBlock\Xml\MetadataProvider:
         arguments:

docs/metadata_generation.md

@@ -58,7 +58,7 @@ the built-in metadata much more flexible than it is now.
 * Every document starts with a terms of use comment. This value can be configured in the `openconext.termsOfUse` ini
   setting.
 * The Metadata documents are signed.
-* The Metadata documents are deemed valid for a period of 86400 seconds (1 day).
+* The Metadata documents are deemed valid for a configurable period (default: 86400 seconds / 1 day), controlled via the `metadata_expiration_time` parameter in `parameters.yml`.
 * The EngineBlock UIInfo elements are generated from the `parameters.yml` config and are translated to English and
 Dutch when possible. Mainly English UI Info will be published.
 * The logo can be specified using the following `parameters.yml` settings

src/OpenConext/EngineBlock/Xml/MetadataRenderer.php

@@ -19,6 +19,7 @@
 namespace OpenConext\EngineBlock\Xml;
 
 use EngineBlock_Saml2_IdGenerator;
+use InvalidArgumentException;
 use OpenConext\EngineBlock\Metadata\Factory\Collection\IdentityProviderEntityCollection;
 use OpenConext\EngineBlock\Metadata\Factory\Helper\IdentityProviderMetadataHelper;
 use OpenConext\EngineBlock\Metadata\Factory\Helper\ServiceProviderMetadataHelper;
@@ -30,14 +31,17 @@
 use OpenConext\EngineBlockBundle\Localization\LanguageSupportProvider;
 use Twig\Environment;
 
+/**
+ * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
+ */
 class MetadataRenderer
 {
     const ID_PREFIX = 'EB';
 
     /**
      * The number of seconds a Metadata document is deemed valid
      */
-    const METADATA_EXPIRATION_TIME = 86400;
+    private int $metadataExpirationTime;
 
     /**
      * @var Environment
@@ -83,15 +87,24 @@ public function __construct(
         KeyPairFactory $keyPairFactory,
         DocumentSigner $documentSigner,
         TimeProvider $timeProvider,
-        string $addRequestedAttributes
+        string $addRequestedAttributes,
+        int $metadataExpirationTime
     ) {
+        if ($metadataExpirationTime <= 0) {
+            throw new InvalidArgumentException(sprintf(
+                'metadataExpirationTime must be a positive integer, %d given',
+                $metadataExpirationTime
+            ));
+        }
+
         $this->languageSupportProvider = $languageSupportProvider;
         $this->twig = $twig;
         $this->samlIdGenerator = $samlIdGenerator;
         $this->keyPairFactory = $keyPairFactory;
         $this->documentSigner = $documentSigner;
         $this->timeProvider = $timeProvider;
         $this->addRequestedAttributes = $addRequestedAttributes;
+        $this->metadataExpirationTime = $metadataExpirationTime;
     }
 
     public function fromServiceProviderEntity(ServiceProviderEntityInterface $sp, string $keyId) : string
@@ -190,6 +203,6 @@ private function renderMetadataXmlIdentityProviderCollection(IdentityProviderEnt
 
     private function getValidUntil(): string
     {
-        return $this->timeProvider->timestamp(self::METADATA_EXPIRATION_TIME);
+        return $this->timeProvider->timestamp($this->metadataExpirationTime);
     }
 }

tests/unit/OpenConext/EngineBlock/Xml/MetadataRendererTest.php

@@ -20,6 +20,7 @@
 use DOMDocument;
 use EngineBlock_Saml2_IdGenerator;
 use Exception;
+use InvalidArgumentException;
 use Mockery as m;
 use Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration;
 use OpenConext\EngineBlock\Metadata\ContactPerson;
@@ -295,8 +296,48 @@ public function metadata_add_no_requested_attributes()
         $this->assertStringNotContainsString($this->getRequestedAttributeXml('attribute3', false), $xml);
     }
 
-    private function buildMetadataRenderer(string $addRequestedAttributes)
+    #[Group('Metadata')]
+    #[Test]
+    public function a_negative_metadata_expiration_time_throws_an_invalid_argument_exception()
+    {
+        $this->expectException(InvalidArgumentException::class);
+        $this->buildMetadataRenderer('all', -1);
+    }
+
+    #[Group('Metadata')]
+    #[Test]
+    public function a_zero_metadata_expiration_time_throws_an_invalid_argument_exception()
     {
+        $this->expectException(InvalidArgumentException::class);
+        $this->buildMetadataRenderer('all', 0);
+    }
+
+    #[Group('Metadata')]
+    #[Test]
+    public function the_configured_metadata_expiration_time_is_reflected_in_the_valid_until_attribute()
+    {
+        $fixedTime = mktime(0, 0, 0, 1, 1, 2026);
+        $expirationTime = 3600;
+
+        $timeProvider = m::mock(TimeProvider::class);
+        $timeProvider->shouldReceive('timestamp')
+            ->andReturnUsing(function ($deltaSeconds) use ($fixedTime) {
+                return gmdate(TimeProvider::TIMESTAMP_FORMAT, $fixedTime + $deltaSeconds);
+            });
+
+        $renderer = $this->buildMetadataRenderer('all', $expirationTime, $timeProvider);
+
+        $expectedValidUntil = gmdate(TimeProvider::TIMESTAMP_FORMAT, $fixedTime + $expirationTime);
+
+        $spXml = $renderer->fromServiceProviderEntity($this->buildSp(), 'default');
+        $this->assertStringContainsString('validUntil="' . $expectedValidUntil . '"', $spXml);
+    }
+
+    private function buildMetadataRenderer(
+        string $addRequestedAttributes,
+        int $metadataExpirationTime = 86400,
+        ?TimeProvider $timeProvider = null
+    ) {
         $basePath = realpath(__DIR__ . '/../../../../../');
 
         $privateKey = new X509PrivateKey($basePath . '/tests/resources/key/engineblock.pem');
@@ -339,8 +380,9 @@ private function buildMetadataRenderer(string $addRequestedAttributes)
             $samlIdGenerator,
             $keyPairFactory,
             $documentSigner,
-            new TimeProvider(),
-            $addRequestedAttributes
+            $timeProvider ?? new TimeProvider(),
+            $addRequestedAttributes,
+            $metadataExpirationTime
         );
     }