Support overriding StepUp EntityId

.gitignore

@@ -20,6 +20,7 @@ bin/ignore_me.php
 .idea
 local-php-security-checker
 /tests/e2e/node_modules
+/tests/.phpunit.result.cache
 /languages/overrides.*.php
 /theme/node_modules
 /theme/.sass-cache

CHANGELOG.md

@@ -5,6 +5,9 @@ We will continue to post relevant release notes on the GitHub release page. More
 
 More information about our release strategy can be found in the [Development Guidelines](https://github.com/OpenConext/OpenConext-engineblock/wiki/Development-Guidelines#release-notes) on the EngineBlock wiki.
 
+## 6.14.0
+* A new feature was added to allow overwriting the internal StepUp auth EntityId 
+
 ## 6.13.0
 
 * Move most HTML from translatable strings into Twig templates, where it

UPGRADING.md

@@ -1,5 +1,24 @@
 # UPGRADE NOTES
 
+## 6.13 -> 6.14
+Previously the SAML EntityID of the EngineBlock SP that was used to do Stepup (SFO) authentications to the Stepup-Gateway 
+always was https://<engineblock.sever.domain.name>/authentication/stepup/metadata. For these authentication the default
+EngineBlock key is always used for signing.
+
+If you'd like to key-rollover the StepUp entity (baked into EngineBlock).
+The key used to sign the SAML AuthnRequests from this SP is the engineblock default key.
+
+To facilitate a rolling configuration update I want the SP entityID that is used for Stepup to be configurable so that at the same time that the engineblock default key is updated, this entityID can be changed. This then allows two entities, with two different keys, to be configured in the Stepup-Gateway.
+
+There are two new parameters that configure this behavior.
+
+1. `feature_stepup_sfo_override_engine_entityid` [bool] enables/disables the feature. Default: disabled
+2. `stepup.sfo.override_engine_entityid` [string] should be set with the Entity ID you'd like to use for the stepup EntityId. Default: ''
+
+The feature flag was added mainly to aid our test suite to easily test this feature.
+
+By default this feature is disabled and the default Entity Id is used for the StepUp entity.
+
 ## 6.12 -> 6.13
 
 Some translatable strings have been changed and "raw" use of HTML in

app/config/config.yml

@@ -27,6 +27,8 @@ open_conext_engine_block:
         eb.enable_sso_notification: "%feature_enable_sso_notification%"
         eb.feature_enable_consent: "%feature_enable_consent%"
         eb.enable_sso_session_cookie: "%feature_enable_sso_session_cookie%"
+        eb.stepup.sfo.override_engine_entityid: "%feature_stepup_sfo_override_engine_entityid%"
+
 
 swiftmailer:
     transport: "%mailer_transport%"

app/config/config_test.yml

@@ -48,6 +48,7 @@ monolog:
             activation_strategy: engineblock.logger.manual_or_error_activation_strategy
             passthru_level: "%logger.fingers_crossed.passthru_level%"
             handler: file
+            channels: ['!event']
         file:
             type:   stream
             path:   "%kernel.logs_dir%/%kernel.environment%.log"

app/config/functional_testing.yml.dist

@@ -3,5 +3,6 @@ parameters:
     router.request_context.scheme: "https"
 
     # Where must we store the writable state of the Mock IdP and Mock SP?
-    idp_fixture_file:   "%kernel.root_dir%/../tmp/fixtures/db/idp.states.php.serialized"
-    sp_fixture_file:    "%kernel.root_dir%/../tmp/fixtures/db/sp.states.php.serialized"
+    idp_fixture_file: '/tmp/eb-fixtures/db/idp.states.php.serialized'
+    sp_fixture_file: '/tmp/eb-fixtures/db/sp.states.php.serialized'
+    stepup.sfo.override_engine_entityid: 'https://engine.vm.openconext.com/new/stepup/metadata'

app/config/parameters.yml.dist

@@ -223,6 +223,7 @@ parameters:
     feature_run_all_manipulations_prior_to_consent: false
     feature_block_user_on_violation: false
     feature_enable_consent: true
+    feature_stepup_sfo_override_engine_entityid: false
 
     ##########################################################################################
     ## PROFILE SETTINGS
@@ -261,6 +262,7 @@ parameters:
     stepup.gateway.sfo.sso_location: 'https://gateway.stepup.vm.openconext.org/second-factor-only/single-sign-on'
     ## The public key from the Stepup Gateway IdP
     stepup.gateway.sfo.key_file: /etc/openconext/engineblock.crt
+    stepup.sfo.override_engine_entityid: 'https://engine.vm.openconext.com/new/stepup/metadata'
 
     ##########################################################################################
     ## THEME SETTINGS

docker/php-fpm/Dockerfile-php72

@@ -3,6 +3,7 @@ FROM ghcr.io/openconext/openconext-basecontainers/php72-apache2-node16-composer2
 RUN a2enmod ssl
 # Copy phpfpm config
 COPY docker/php-fpm/app.ini /usr/local/etc/php/conf.d/
+RUN rm -rf /etc/apache2/sites-enabled/*
 COPY docker/php-fpm/apache2.conf /etc/apache2/sites-enabled/
 RUN chown -R www-data: /var/www/
 WORKDIR /opt/openconext/OpenConext-engineblock

library/EngineBlock/Application/DiContainer.php

@@ -17,7 +17,7 @@
  */
 
 use Doctrine\ORM\EntityManager;
-use OpenConext\EngineBlock\Metadata\Factory\Factory\ServiceProviderFactor;
+use OpenConext\EngineBlock\Metadata\Factory\Factory\ServiceProviderFactory;
 use OpenConext\EngineBlock\Metadata\LoaRepository;
 use OpenConext\EngineBlock\Metadata\MetadataRepository\MetadataRepositoryInterface;
 use OpenConext\EngineBlock\Service\MfaHelperInterface;
@@ -521,6 +521,12 @@ protected function getStepupEndpoint()
         return $this->container->get('engineblock.configuration.stepup.endpoint');
     }
 
+    /** @return string */
+    public function getStepupEntityIdOverrideValue()
+    {
+        return $this->container->getParameter('stepup.sfo.override_engine_entityid');
+    }
+
     public function getCookieDomain()
     {
         return $this->container->getParameter('cookie.locale.domain');

library/EngineBlock/Corto/Module/Service/StepupAssertionConsumer.php

@@ -73,12 +73,15 @@ public function __construct(
      */
     public function serve($serviceName, Request $httpRequest)
     {
-        $serviceEntityId = $this->_server->getUrl('stepupMetadataService');
-        $expectedDestination = $this->_server->getUrl('stepupAssertionConsumerService');
-
         $application = EngineBlock_ApplicationSingleton::getInstance();
         $log = $application->getLogInstance();
 
+        // EngineBlock will verify the incoming assertion's Audience which will
+        // be set to the entityID it used on the outgoing AuthNRequest, so this
+        // place will also need to handle the override if present.
+        $serviceEntityId = $this->determineRemoteSpEntityId();
+        $expectedDestination = $this->_server->getUrl('stepupAssertionConsumerService');
+
         $checkResponseSignature = true;
         try {
             $receivedResponse = $this->_server->getBindingsModule()->receiveResponse($serviceEntityId, $expectedDestination);
@@ -138,6 +141,30 @@ public function serve($serviceName, Request $httpRequest)
         return;
     }
 
+    /**
+     * Returns the `stepupMetadataService` if no override is defined.
+     * To define an override (for StepUp key rollover) configure:
+     * `eb.stepup.sfo.override_engine_entityid`. See UPGRADING.md (6.13 -> 6.14)
+     * for details.
+     *
+     * @return string
+     */
+    private function determineRemoteSpEntityId()
+    {
+        $application = EngineBlock_ApplicationSingleton::getInstance();
+        $container = $application->getDiContainer();
+        $entityIdOverrideValue = $container->getStepupEntityIdOverrideValue();
+        $features = $container->getFeatureConfiguration();
+        $isConfigured = $features->hasFeature('eb.stepup.sfo.override_engine_entityid');
+        $isEnabled = $features->isEnabled('eb.stepup.sfo.override_engine_entityid');
+
+        $serviceEntityId = $this->_server->getUrl('stepupMetadataService');
+        if ($isEnabled && $isConfigured) {
+            return $entityIdOverrideValue;
+        }
+        return $serviceEntityId;
+    }
+
     /**
      * @return AuthenticationState
      */

library/EngineBlock/Corto/ProxyServer.php

@@ -16,6 +16,7 @@
  * limitations under the License.
  */
 
+use OpenConext\EngineBlock\Exception\MissingParameterException;
 use OpenConext\EngineBlock\Metadata\Entity\AbstractRole;
 use OpenConext\EngineBlock\Metadata\Entity\IdentityProvider;
 use OpenConext\EngineBlock\Metadata\Entity\ServiceProvider;
@@ -487,6 +488,32 @@ public function sendStepupAuthenticationRequest(
         $nameIdOverwrite->setFormat(Constants::NAMEID_UNSPECIFIED);
         $sspMessage->setNameId($nameIdOverwrite);
 
+        // See: UPGRADING.md -> ## 6.13 -> 6.14
+        $container = EngineBlock_ApplicationSingleton::getInstance()->getDiContainer();
+        $entityIdOverrideValue = $container->getStepupEntityIdOverrideValue();
+        $features = $container->getFeatureConfiguration();
+        $isConfigured = $features->hasFeature('eb.stepup.sfo.override_engine_entityid');
+        $isEnabled = $features->isEnabled('eb.stepup.sfo.override_engine_entityid');
+
+        if ($isEnabled && $isConfigured) {
+            if (empty($entityIdOverrideValue)) {
+                throw new MissingParameterException(
+                    'When feature "feature_stepup_sfo_override_engine_entityid" is enabled,
+                     you must provide the "stepup.sfo.override_engine_entityid" parameter.'
+                );
+            }
+            $this->_logger->notice(
+                sprintf(
+                    'Feature eb.stepup.sfo.override_engine_entityid is enabled, overriding the Issuer of the AR to the ' .
+                    'StepUp Gateway. Updated the Issuer to "%s"',
+                    $entityIdOverrideValue
+                )
+            );
+            $issuer = new Issuer();
+            $issuer->setValue($entityIdOverrideValue);
+            $sspMessage->setIssuer($issuer);
+        }
+
         // Link with the original Request
         $authnRequestRepository = new EngineBlock_Saml2_AuthnRequestSessionRepository($this->_logger);
         $authnRequestRepository->store($spRequest);

src/OpenConext/EngineBlock/Metadata/Factory/Factory/ServiceProviderFactory.php

@@ -19,6 +19,7 @@
 namespace OpenConext\EngineBlock\Metadata\Factory\Factory;
 
 use EngineBlock_Attributes_Metadata as AttributesMetadata;
+use OpenConext\EngineBlock\Exception\MissingParameterException;
 use OpenConext\EngineBlock\Metadata\Entity\ServiceProvider;
 use OpenConext\EngineBlock\Metadata\Factory\Adapter\ServiceProviderEntity;
 use OpenConext\EngineBlock\Metadata\Factory\Decorator\EngineBlockServiceProvider;
@@ -28,11 +29,13 @@
 use OpenConext\EngineBlock\Metadata\Factory\ValueObject\EngineBlockConfiguration;
 use OpenConext\EngineBlock\Metadata\Mdui;
 use OpenConext\EngineBlock\Metadata\X509\KeyPairFactory;
+use OpenConext\EngineBlockBundle\Configuration\FeatureConfigurationInterface;
 use OpenConext\EngineBlockBundle\Url\UrlProvider;
 
 /**
  * This factory is used for instantiating an entity with the required adapters and/or decorators set.
  * It also makes sure that static, internally used, entities can be generated without the use of the database.
+ * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
 class ServiceProviderFactory
 {
@@ -55,16 +58,30 @@ class ServiceProviderFactory
      */
     private $urlProvider;
 
+    /**
+     * @var string
+     */
+    private $entityIdOverrideValue;
+
+    /**
+     * @var FeatureConfigurationInterface
+     */
+    private $featureConfiguration;
+
     public function __construct(
         AttributesMetadata $attributes,
         KeyPairFactory $keyPairFactory,
         EngineBlockConfiguration $engineBlockConfiguration,
-        UrlProvider $urlProvider
+        UrlProvider $urlProvider,
+        FeatureConfigurationInterface $featureConfiguration,
+        string $entityIdOverrideValue
     ) {
         $this->attributes = $attributes;
         $this->keyPairFactory = $keyPairFactory;
         $this->engineBlockConfiguration = $engineBlockConfiguration;
         $this->urlProvider = $urlProvider;
+        $this->featureConfiguration = $featureConfiguration;
+        $this->entityIdOverrideValue = $entityIdOverrideValue;
     }
 
     public function createEngineBlockEntityFrom(string $keyId): ServiceProviderEntityInterface
@@ -86,8 +103,20 @@ public function createEngineBlockEntityFrom(string $keyId): ServiceProviderEntit
 
     public function createStepupEntityFrom(string $keyId): ServiceProviderEntityInterface
     {
+        $isConfigured = $this->featureConfiguration->hasFeature('eb.stepup.sfo.override_engine_entityid');
+        $isEnabled = $this->featureConfiguration->isEnabled('eb.stepup.sfo.override_engine_entityid');
         $entityId = $this->urlProvider->getUrl('metadata_stepup', false, null, null);
 
+        if ($isEnabled && $isConfigured) {
+            if (empty($this->entityIdOverrideValue)) {
+                throw new MissingParameterException(
+                    'When feature "feature_stepup_sfo_override_engine_entityid" is enabled, you must provide the '.
+                    '"stepup.sfo.override_engine_entityid" parameter.'
+                );
+            }
+            $entityId = $this->entityIdOverrideValue;
+        }
+
         $entity = $this->buildServiceProviderOrmEntity($entityId);
 
         return new ServiceProviderStepup( // Add stepup data

src/OpenConext/EngineBlock/Stepup/StepupEntityFactory.php

@@ -18,7 +18,6 @@
 namespace OpenConext\EngineBlock\Stepup;
 
 use EngineBlock_X509_CertificateFactory;
-use OpenConext\EngineBlock\Metadata\EmptyMduiElement;
 use OpenConext\EngineBlock\Metadata\Entity\IdentityProvider;
 use OpenConext\EngineBlock\Metadata\Entity\ServiceProvider;
 use OpenConext\EngineBlock\Metadata\IndexedService;
@@ -29,7 +28,6 @@
 
 class StepupEntityFactory
 {
-
     /**
      * @throws \EngineBlock_Exception
      */

src/OpenConext/EngineBlockBundle/Configuration/TestFeatureConfiguration.php

@@ -45,6 +45,7 @@ public function __construct()
         $this->setFeature(new Feature('eb.enable_sso_notification', false));
         $this->setFeature(new Feature('eb.feature_enable_consent', true));
         $this->setFeature(new Feature('eb.enable_sso_session_cookie', true));
+        $this->setFeature(new Feature('eb.stepup.sfo.override_engine_entityid', false));
     }
 
     public function setFeature(Feature $feature): void

src/OpenConext/EngineBlockBundle/Resources/config/services.yml

@@ -129,6 +129,8 @@ services:
             - '@OpenConext\EngineBlock\Metadata\X509\KeyPairFactory'
             - '@OpenConext\EngineBlock\Metadata\Factory\ValueObject\EngineBlockConfiguration'
             - '@engineblock.url_provider'
+            - '@engineblock.features'
+            - '%stepup.sfo.override_engine_entityid%'
 
     OpenConext\EngineBlock\Metadata\Factory\ValueObject\EngineBlockConfiguration:
         public: false

src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Context/EngineBlockContext.php

@@ -18,14 +18,11 @@
 
 namespace OpenConext\EngineBlockFunctionalTestingBundle\Features\Context;
 
-use Behat\Behat\Tester\Exception\PendingException;
 use Behat\Gherkin\Node\TableNode;
 use Behat\Mink\Exception\ExpectationException;
 use DOMDocument;
 use DOMElement;
 use DOMXPath;
-use Ingenerator\BehatTableAssert\AssertTable;
-use Ingenerator\BehatTableAssert\TableParser\HTMLTable;
 use OpenConext\EngineBlockFunctionalTestingBundle\Fixtures\FunctionalTestingAttributeAggregationClient;
 use OpenConext\EngineBlockFunctionalTestingBundle\Fixtures\FunctionalTestingAuthenticationLoopGuard;
 use OpenConext\EngineBlockFunctionalTestingBundle\Fixtures\FunctionalTestingFeatureConfiguration;
@@ -39,10 +36,6 @@
 use RuntimeException;
 use SAML2\Constants;
 use SAML2\DOMDocumentFactory;
-use function assertStringNotMatchesFormat;
-use function assertStringStartsWith;
-use function preg_match;
-use function sprintf;
 
 /**
  * @SuppressWarnings(PHPMD.TooManyPublicMethods) Both set up and tasks can be a lot...

src/OpenConext/EngineBlockFunctionalTestingBundle/Features/StepupKeyRollover.feature

@@ -0,0 +1,43 @@
+Feature:
+  In order to facilitate a rolling configuration update
+  As EngineBlock
+  I want the SP entityID that is used for Stepup auth to be configurable so that at the same time
+  that the EngineBlock default key is updated, this entityID can be changed.
+  This then allows two entities, with two different keys, to be configured in the Stepup-Gateway
+
+  Background:
+    Given an EngineBlock instance on "vm.openconext.org"
+    And no registered SPs
+    And no registered Idps
+    And an Identity Provider named "SSO-IdP"
+    And a Service Provider named "SSO-SP"
+    And an Identity Provider named "Dummy-IdP"
+    And a Service Provider named "Dummy-SP"
+    And a Service Provider named "Proxy-SP"
+
+  Scenario: When stepup.sfo.override_engine_entityid is not configured, stepup/metadata should show default EntityId
+    Given feature "eb.stepup.sfo.override_engine_entityid" is disabled
+    When I go to Engineblock URL "/authentication/stepup/metadata"
+    Then the response should match xpath '//md:EntityDescriptor[@entityID="https://engine.vm.openconext.org/authentication/stepup/metadata"]'
+
+  Scenario: When stepup.sfo.override_engine_entityid is configured with a valid EntityId, stepup/metadata should show that EntityId
+    Given feature "eb.stepup.sfo.override_engine_entityid" is enabled
+    When I go to Engineblock URL "/authentication/stepup/metadata"
+    Then the response should match xpath '//md:EntityDescriptor[@entityID="https://engine.vm.openconext.com/new/stepup/metadata"]'
+
+  # Note that we can not ascertain programatically if the Issuer is updated as this is an internal
+  # redirect response where we can not easily intervene with the browser (we would need to disable
+  # auto-following of redirects). This test does hit the code, and proves that the authentication
+  # is not broken by it.
+  Scenario: When stepup.sfo.override_engine_entityid is configured, the the Issuer is updated
+    Given feature "eb.stepup.sfo.override_engine_entityid" is enabled
+    And the SP "SSO-SP" requires Stepup LoA "http://vm.openconext.org/assurance/loa2"
+    When I log in at "SSO-SP"
+    And I select "SSO-IdP" on the WAYF
+    And I pass through EngineBlock
+    # This is where the Issuer is overridden. See: \EngineBlock_Corto_ProxyServer::sendStepupAuthenticationRequest
+    And I pass through the IdP
+    And Stepup will successfully verify a user
+    And I give my consent
+    And I pass through EngineBlock
+    Then the url should match "/functional-testing/SSO-SP/acs"

src/OpenConext/EngineBlockFunctionalTestingBundle/Fixtures/FunctionalTestingFeatureConfiguration.php

@@ -42,7 +42,7 @@ final class FunctionalTestingFeatureConfiguration implements FeatureConfiguratio
      */
     private $dataStore;
 
-    public function __construct(TestFeatureConfiguration $featureConfiguration, AbstractDataStore $dataStore)
+    public function __construct(FeatureConfigurationInterface $featureConfiguration, AbstractDataStore $dataStore)
     {
         $this->featureConfiguration = $featureConfiguration;
         $this->dataStore            = $dataStore;