diff --git a/.github/workflows/test-integration.yml b/.github/workflows/test-integration.yml index e1af0be942..bb23e50c75 100644 --- a/.github/workflows/test-integration.yml +++ b/.github/workflows/test-integration.yml @@ -35,7 +35,6 @@ jobs: ./app/console cache:clear --env=ci && \ cd theme && CYPRESS_INSTALL_BINARY=0 yarn install --frozen-lockfile && EB_THEME=skeune yarn build ' - - name: Run code quality tests if: always() run: | diff --git a/.gitignore b/.gitignore index e616e526a4..3a30032dc1 100644 --- a/.gitignore +++ b/.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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 4bdc6f8be9..b4d3c51310 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/UPGRADING.md b/UPGRADING.md index add15b5fd9..cc61cebbbe 100644 --- a/UPGRADING.md +++ b/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:///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 diff --git a/app/config/config.yml b/app/config/config.yml index 00dc0aefb4..5079db3c20 100644 --- a/app/config/config.yml +++ b/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%" diff --git a/app/config/config_test.yml b/app/config/config_test.yml index 74e1989631..8fff176bbf 100644 --- a/app/config/config_test.yml +++ b/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" diff --git a/app/config/functional_testing.yml.dist b/app/config/functional_testing.yml.dist index 5a303c6cab..c365ff0294 100644 --- a/app/config/functional_testing.yml.dist +++ b/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' diff --git a/app/config/parameters.yml.dist b/app/config/parameters.yml.dist index 42b9c0bfb4..ad4fe2b23a 100644 --- a/app/config/parameters.yml.dist +++ b/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 diff --git a/src/OpenConext/EngineBlock/Metadata/Factory/Factory/ServiceProviderFactory.php b/src/OpenConext/EngineBlock/Metadata/Factory/Factory/ServiceProviderFactory.php index 4402efbd0e..f99e18d70a 100644 --- a/src/OpenConext/EngineBlock/Metadata/Factory/Factory/ServiceProviderFactory.php +++ b/src/OpenConext/EngineBlock/Metadata/Factory/Factory/ServiceProviderFactory.php @@ -28,6 +28,7 @@ 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; /** @@ -55,16 +56,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 +101,14 @@ 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) { + $entityId = $this->entityIdOverrideValue; + } + $entity = $this->buildServiceProviderOrmEntity($entityId); return new ServiceProviderStepup( // Add stepup data diff --git a/src/OpenConext/EngineBlockBundle/Configuration/TestFeatureConfiguration.php b/src/OpenConext/EngineBlockBundle/Configuration/TestFeatureConfiguration.php index 4c1dace502..7d4167991c 100644 --- a/src/OpenConext/EngineBlockBundle/Configuration/TestFeatureConfiguration.php +++ b/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 diff --git a/src/OpenConext/EngineBlockBundle/Resources/config/services.yml b/src/OpenConext/EngineBlockBundle/Resources/config/services.yml index 9c75b6704e..c7bf6582d6 100644 --- a/src/OpenConext/EngineBlockBundle/Resources/config/services.yml +++ b/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 diff --git a/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Context/EngineBlockContext.php b/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Context/EngineBlockContext.php index 7e388c5edf..ada66f7efc 100644 --- a/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Context/EngineBlockContext.php +++ b/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... diff --git a/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/StepupKeyRollover.feature b/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/StepupKeyRollover.feature new file mode 100644 index 0000000000..895735f341 --- /dev/null +++ b/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/StepupKeyRollover.feature @@ -0,0 +1,35 @@ +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"]' + + Scenario: When stepup.sfo.override_engine_entityid is configured, the the Issuer is updated + Given the SP "SSO-SP" requires Stepup LoA "http://vm.openconext.org/assurance/loa2" + And feature "eb.stepup.sfo.override_engine_entityid" is enabled + When I log in at "SSO-SP" + And I select "SSO-IdP" on the WAYF + Then the response should match xpath '//md:EntityDescriptor[@entityID="https://engine.vm.openconext.com/new/stepup/metadata"]' + + diff --git a/src/OpenConext/EngineBlockFunctionalTestingBundle/Resources/config/services.yml b/src/OpenConext/EngineBlockFunctionalTestingBundle/Resources/config/services.yml index d8fadd3cce..b4031c05fe 100644 --- a/src/OpenConext/EngineBlockFunctionalTestingBundle/Resources/config/services.yml +++ b/src/OpenConext/EngineBlockFunctionalTestingBundle/Resources/config/services.yml @@ -1,12 +1,12 @@ parameters: - engineblock.functional_testing.service_registry_data_store.dir: "%kernel.root_dir%/../tmp/eb-fixtures/metadata-push/" - engineblock.functional_testing.service_registry_data_store.file: "%kernel.root_dir%/../tmp/eb-fixtures/metadata-push/entities" - engineblock.functional_testing.features_data_store.file: "%kernel.root_dir%/../tmp/eb-fixtures/features.json" - engineblock.functional_testing.authentication_loop_guard_data_store.file: "%kernel.root_dir%/../tmp/eb-fixtures/authentication-loop-guard.json" - engineblock.functional_testing.pdp_data_store.file: "%kernel.root_dir%/../tmp/eb-fixtures/policy_decision.json" - engineblock.functional_testing.attribute_aggregation_data_store.file: "%kernel.root_dir%/../tmp/eb-fixtures/attribute_aggregation.json" - engineblock.functional_testing.stepup_gateway_mock_data_store.file: "%kernel.root_dir%/../tmp/eb-fixtures/stepup_gateway_mock.json" - engineblock.functional_testing.translator_mock_data_store.file: "%kernel.root_dir%/../tmp/eb-fixtures/translator_mock.json" + engineblock.functional_testing.service_registry_data_store.dir: "/tmp/eb-fixtures/metadata-push/" + engineblock.functional_testing.service_registry_data_store.file: "/tmp/eb-fixtures/metadata-push/entities" + engineblock.functional_testing.features_data_store.file: "/tmp/eb-fixtures/features.json" + engineblock.functional_testing.authentication_loop_guard_data_store.file: "/tmp/eb-fixtures/authentication-loop-guard.json" + engineblock.functional_testing.pdp_data_store.file: "/tmp/eb-fixtures/policy_decision.json" + engineblock.functional_testing.attribute_aggregation_data_store.file: "/tmp/eb-fixtures/attribute_aggregation.json" + engineblock.functional_testing.stepup_gateway_mock_data_store.file: "/tmp/eb-fixtures/stepup_gateway_mock.json" + engineblock.functional_testing.translator_mock_data_store.file: "/tmp/eb-fixtures/translator_mock.json" services: engineblock.functional_testing.service.engine_block: @@ -109,3 +109,13 @@ services: class: OpenConext\EngineBlockBundle\Configuration\ErrorFeedbackConfiguration arguments: - "@engineblock.functional_testing.mock.translator" + + engineblock.factory.service_provider_factory: + class: OpenConext\EngineBlock\Metadata\Factory\Factory\ServiceProviderFactory + arguments: + - '@engineblock.compat.metadata.definitions' + - '@OpenConext\EngineBlock\Metadata\X509\KeyPairFactory' + - '@OpenConext\EngineBlock\Metadata\Factory\ValueObject\EngineBlockConfiguration' + - '@engineblock.url_provider' + - '@engineblock.functional_testing.fixture.features' + - '%stepup.sfo.override_engine_entityid%' diff --git a/tests/behat.yml b/tests/behat.yml index 273730cb68..0791a01f37 100644 --- a/tests/behat.yml +++ b/tests/behat.yml @@ -87,4 +87,3 @@ default: - "--no-sandbox" - "--disable-dev-shm-usage" Behat\Symfony2Extension: ~ -