Skip to content

Commit

Permalink
Support overriding StepUp EntityId
Browse files Browse the repository at this point in the history
  • Loading branch information
MKodde committed Nov 21, 2023
1 parent f7e664d commit eabfc29
Show file tree
Hide file tree
Showing 16 changed files with 238 additions and 42 deletions.
1 change: 0 additions & 1 deletion .github/workflows/test-integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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: |
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 19 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 2 additions & 0 deletions app/config/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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%"
Expand Down
1 change: 1 addition & 0 deletions app/config/config_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
5 changes: 3 additions & 2 deletions app/config/functional_testing.yml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -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'
2 changes: 2 additions & 0 deletions app/config/parameters.yml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -28,6 +29,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;

/**
Expand Down Expand Up @@ -55,16 +57,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
Expand All @@ -86,8 +102,17 @@ 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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...
Expand Down
Original file line number Diff line number Diff line change
@@ -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"]'


Original file line number Diff line number Diff line change
@@ -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:
Expand Down Expand Up @@ -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%'
1 change: 0 additions & 1 deletion tests/behat.yml
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,3 @@ default:
- "--no-sandbox"
- "--disable-dev-shm-usage"
Behat\Symfony2Extension: ~

Loading

0 comments on commit eabfc29

Please sign in to comment.