From 49d2b197a827518d8c8c7efbd2ab00ecf319ddef Mon Sep 17 00:00:00 2001 From: Kay Joosten Date: Fri, 10 Jul 2026 13:10:46 +0200 Subject: [PATCH] Accept nullable service_name in SP config Gateway now expects an optional service_name in pushed SP configuration so it can display a friendly name during authentication. Middleware still rejected that key because the validator only allowed the older field set, and fresh CI installs had become flaky because phpcpd-shim was fetched from a dead upstream Git host. Allow service_name as an optional nullable field in the service provider validator and add regression coverage for valid and invalid configurations. Replace the dead phpcpd shim with the maintained phpcpd-next package so fresh installs keep working reliably. Resolves: #589 --- composer.json | 2 +- composer.lock | 118 +++++++++++------- .../ConfigurationControllerTest.php | 14 +++ .../invalid_sp_service_name.php | 41 ++++++ .../ServiceProviderConfigurationValidator.php | 14 +++ 5 files changed, 145 insertions(+), 44 deletions(-) create mode 100644 src/Surfnet/StepupMiddleware/ManagementBundle/Tests/Validator/Fixtures/invalid_configuration/invalid_sp_service_name.php diff --git a/composer.json b/composer.json index 9e455027d..63cfe510f 100644 --- a/composer.json +++ b/composer.json @@ -57,11 +57,11 @@ "require-dev": { "doctrine/data-fixtures": "^2.2", "doctrine/doctrine-fixtures-bundle": "^4.3", - "irstea/phpcpd-shim": "^6.0", "liip/test-fixtures-bundle": "^3.6", "malukenho/docheader": "^1.1", "mockery/mockery": "^1.6.12", "overtrue/phplint": ">=9.5.6", + "phpcpd-next/phpcpd": "^1.1", "phpmd/phpmd": "^2.15", "phpstan/phpstan": "^2.0", "phpstan/phpstan-doctrine": "^2.0", diff --git a/composer.lock b/composer.lock index 6fc07dadf..abcbd57f7 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "c6c7c29f8f3077e6a05ed8f956c70543", + "content-hash": "df942aa938b0837e2ba6263a2df3d8a2", "packages": [ { "name": "beberlei/assert", @@ -9281,48 +9281,6 @@ }, "time": "2025-04-30T06:54:44+00:00" }, - { - "name": "irstea/phpcpd-shim", - "version": "6.0.3", - "source": { - "type": "git", - "url": "https://gitlab.irstea.fr/pole-is/tools/phpcpd-shim.git", - "reference": "5829d11d1379fd92176b7e2105060c7f09f4fdda" - }, - "require": { - "ext-dom": "*", - "php": ">=7.3" - }, - "replace": { - "sebastian/phpcpd": "self.version" - }, - "bin": [ - "phpcpd" - ], - "type": "library", - "autoload": { - "exclude-from-classmap": [ - "phpcpd" - ] - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "BSD-3-Clause" - ], - "authors": [ - { - "name": "Sebastian Bergmann", - "email": "sebastian@phpunit.de", - "role": "lead" - } - ], - "description": "Shim repository for sebastian/phpcpd", - "homepage": "https://github.com/sebastianbergmann/phpcpd", - "keywords": [ - "shim" - ], - "time": "2020-12-08T03:20:09+00:00" - }, { "name": "liip/test-fixtures-bundle", "version": "3.7.0", @@ -9945,6 +9903,80 @@ }, "time": "2022-02-21T01:04:05+00:00" }, + { + "name": "phpcpd-next/phpcpd", + "version": "v1.1", + "source": { + "type": "git", + "url": "https://github.com/phpcpd-next/phpcpd.git", + "reference": "4184c2eec6c04526222a59ee4638b8bcc6128f23" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/phpcpd-next/phpcpd/zipball/4184c2eec6c04526222a59ee4638b8bcc6128f23", + "reference": "4184c2eec6c04526222a59ee4638b8bcc6128f23", + "shasum": "" + }, + "require": { + "ext-dom": "*", + "ext-mbstring": "*", + "php": ">=8.5" + }, + "require-dev": { + "friendsofphp/php-cs-fixer": "^3.64", + "phpstan/phpstan": "^2.2", + "phpunit/phpunit": "^11.0 || ^12.0 || ^13.0", + "rector/rector": "^2.0" + }, + "suggest": { + "phpunit/phpunit": "To use the PHPUnit integration (AssertNoDuplication trait / DuplicationConstraint) for duplication-as-a-test" + }, + "bin": [ + "phpcpd" + ], + "type": "library", + "autoload": { + "psr-4": { + "LucianoPereira\\PhpcpdNext\\": [ + "src/", + "src/CLI/", + "src/Exceptions/" + ], + "LucianoPereira\\PhpcpdNext\\PHPUnit\\": "integration/phpunit/src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "BSD-3-Clause" + ], + "authors": [ + { + "name": "Sebastian Bergmann", + "email": "sebastian@phpunit.de", + "role": "Original author" + }, + { + "name": "Luciano Federico Pereira", + "role": "Fork maintainer" + } + ], + "description": "phpcpd-next — Copy/Paste (clone) detector for PHP 8.5+ with Type-1/2/3 detection. The maintained successor to the archived sebastianbergmann/phpcpd.", + "homepage": "https://github.com/phpcpd-next/phpcpd", + "keywords": [ + "ci", + "clone-detection", + "code-quality", + "copy-paste-detector", + "duplication", + "phpcpd", + "static-analysis" + ], + "support": { + "issues": "https://github.com/phpcpd-next/phpcpd/issues", + "source": "https://github.com/phpcpd-next/phpcpd" + }, + "time": "2026-06-28T08:39:48+00:00" + }, { "name": "phpmd/phpmd", "version": "2.15.0", diff --git a/src/Surfnet/StepupMiddleware/ManagementBundle/Tests/Controller/ConfigurationControllerTest.php b/src/Surfnet/StepupMiddleware/ManagementBundle/Tests/Controller/ConfigurationControllerTest.php index 958e5bb38..8bcf15d4f 100644 --- a/src/Surfnet/StepupMiddleware/ManagementBundle/Tests/Controller/ConfigurationControllerTest.php +++ b/src/Surfnet/StepupMiddleware/ManagementBundle/Tests/Controller/ConfigurationControllerTest.php @@ -218,6 +218,20 @@ public function validPushWithOnlyGatewayIsAccepted(): void ]); } + #[Test] + #[Group('management')] + public function validPushWithServiceNameIsAccepted(): void + { + $this->pushConfiguration([ + 'gateway' => [ + 'identity_providers' => [], + 'service_providers' => [ + array_merge(self::minimalServiceProvider(), ['service_name' => 'My Service']), + ], + ], + ]); + } + #[Test] #[Group('management')] public function pushWithSraaAndEmailTemplatesIsSilentlyAccepted(): void diff --git a/src/Surfnet/StepupMiddleware/ManagementBundle/Tests/Validator/Fixtures/invalid_configuration/invalid_sp_service_name.php b/src/Surfnet/StepupMiddleware/ManagementBundle/Tests/Validator/Fixtures/invalid_configuration/invalid_sp_service_name.php new file mode 100644 index 000000000..9827d08c8 --- /dev/null +++ b/src/Surfnet/StepupMiddleware/ManagementBundle/Tests/Validator/Fixtures/invalid_configuration/invalid_sp_service_name.php @@ -0,0 +1,41 @@ + 'gateway.service_providers[0].service_name', + 'configuration' => [ + 'gateway' => [ + 'identity_providers' => [], + 'service_providers' => [ + [ + "entity_id" => "https://entity.tld/id", + "public_key" => "MIIE...", + "acs" => ["https://entity.tld/consume-assertion"], + "loa" => [ + "__default__" => "https://entity.tld/authentication/loa2", + ], + "second_factor_only" => false, + "second_factor_only_nameid_patterns" => [], + "assertion_encryption_enabled" => false, + "blacklisted_encryption_algorithms" => [], + "service_name" => ['not', 'a', 'string'], + ], + ], + ], + ], +]; diff --git a/src/Surfnet/StepupMiddleware/ManagementBundle/Validator/ServiceProviderConfigurationValidator.php b/src/Surfnet/StepupMiddleware/ManagementBundle/Validator/ServiceProviderConfigurationValidator.php index 3da30b8a0..3cc1603f8 100644 --- a/src/Surfnet/StepupMiddleware/ManagementBundle/Validator/ServiceProviderConfigurationValidator.php +++ b/src/Surfnet/StepupMiddleware/ManagementBundle/Validator/ServiceProviderConfigurationValidator.php @@ -40,6 +40,7 @@ public function validate(array $configuration, string $propertyPath): void 'use_pdp', 'allow_sso_on_2fa', 'set_sso_cookie_on_2fa', + 'service_name', ]; if (empty($configuration['use_pdp'])) { @@ -54,6 +55,10 @@ public function validate(array $configuration, string $propertyPath): void $configuration['set_sso_cookie_on_2fa'] = false; } + if (!array_key_exists('service_name', $configuration)) { + $configuration['service_name'] = null; + } + StepupAssert::keysMatch( $configuration, $requiredProperties, @@ -91,6 +96,7 @@ public function validate(array $configuration, string $propertyPath): void $this->validateBooleanValue($configuration, 'use_pdp', $propertyPath); $this->validateBooleanValue($configuration, 'allow_sso_on_2fa', $propertyPath); $this->validateBooleanValue($configuration, 'set_sso_cookie_on_2fa', $propertyPath); + $this->validateNullableStringValue($configuration, 'service_name', $propertyPath); } private function validateStringValue(array $configuration, string $name, string $propertyPath): void @@ -98,6 +104,14 @@ private function validateStringValue(array $configuration, string $name, string Assertion::string($configuration[$name], 'value must be a string', $propertyPath . '.' . $name); } + /** + * @param array $configuration + */ + private function validateNullableStringValue(array $configuration, string $name, string $propertyPath): void + { + Assertion::nullOrString($configuration[$name], 'value must be a string or null', $propertyPath . '.' . $name); + } + private function validateStringValues(array $configuration, string $name, string $propertyPath): void { Assertion::isArray($configuration[$name], 'value must be an array', $propertyPath . '.' . $name);