diff --git a/composer.json b/composer.json index 082080c314..ced0a2f812 100644 --- a/composer.json +++ b/composer.json @@ -36,6 +36,7 @@ "pimple/pimple": "^3.6", "ramsey/uuid": "^4.9.1", "robrichards/xmlseclibs": "^3.1.3", + "simplesamlphp/assert": "^1.9.1", "simplesamlphp/saml2": "^4.19", "symfony/asset": "^7.4", "symfony/console": "^7.4", diff --git a/composer.lock b/composer.lock index 21cb23e722..a0b1b8abcf 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": "19eeea20ede2f61534eaf59f084f20b4", + "content-hash": "f6010857a879802cd455d14a4c376463", "packages": [ { "name": "beberlei/assert", @@ -2740,6 +2740,65 @@ }, "time": "2026-03-13T10:31:56+00:00" }, + { + "name": "simplesamlphp/assert", + "version": "v1.9.1", + "source": { + "type": "git", + "url": "https://github.com/simplesamlphp/assert.git", + "reference": "d0e7b9c12e4f613c6f32e1242cc08838cdeb39d3" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/simplesamlphp/assert/zipball/d0e7b9c12e4f613c6f32e1242cc08838cdeb39d3", + "reference": "d0e7b9c12e4f613c6f32e1242cc08838cdeb39d3", + "shasum": "" + }, + "require": { + "ext-date": "*", + "ext-filter": "*", + "ext-pcre": "*", + "ext-spl": "*", + "guzzlehttp/psr7": "~2.8.0", + "php": "^8.2", + "webmozart/assert": "~1.12.0" + }, + "require-dev": { + "ext-intl": "*", + "simplesamlphp/simplesamlphp-test-framework": "~1.10.2" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "v1.10.x-dev" + } + }, + "autoload": { + "psr-4": { + "SimpleSAML\\Assert\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "LGPL-2.1-or-later" + ], + "authors": [ + { + "name": "Tim van Dijen", + "email": "tvdijen@gmail.com" + }, + { + "name": "Jaime Perez Crespo", + "email": "jaimepc@gmail.com" + } + ], + "description": "A wrapper around webmozart/assert to make it useful beyond checking method arguments", + "support": { + "issues": "https://github.com/simplesamlphp/assert/issues", + "source": "https://github.com/simplesamlphp/assert/tree/v1.9.1" + }, + "time": "2025-10-20T22:29:33+00:00" + }, { "name": "simplesamlphp/saml2", "version": "v4.19.1", diff --git a/library/EngineBlock/Attributes/Validator/Type.php b/library/EngineBlock/Attributes/Validator/Type.php index 4b65b79b5f..1f7030f7ed 100644 --- a/library/EngineBlock/Attributes/Validator/Type.php +++ b/library/EngineBlock/Attributes/Validator/Type.php @@ -16,6 +16,9 @@ * limitations under the License. */ +use SimpleSAML\Assert\Assert; +use SimpleSAML\Assert\AssertionFailedException; + class EngineBlock_Attributes_Validator_Type extends EngineBlock_Attributes_Validator_Abstract { const ERROR_ATTRIBUTE_VALIDATOR_URI = 'error_attribute_validator_type_uri'; @@ -75,7 +78,9 @@ public function validate(array $attributes) case 'URL': foreach ($attributeValues as $attributeValue) { - if (filter_var($attributeValue, FILTER_VALIDATE_URL) === false) { + try { + Assert::validURL($attributeValue); + } catch (AssertionFailedException $e) { $this->_messages[] = array( self::ERROR_ATTRIBUTE_VALIDATOR_URL, $this->_attributeName, diff --git a/library/EngineBlock/Validator/Uri.php b/library/EngineBlock/Validator/Uri.php index b65de2a4ab..e34acd9ebb 100644 --- a/library/EngineBlock/Validator/Uri.php +++ b/library/EngineBlock/Validator/Uri.php @@ -16,12 +16,14 @@ * limitations under the License. */ +use SimpleSAML\Assert\Assert; +use SimpleSAML\Assert\AssertionFailedException; + /** - * Validate URIs according to RFC-3986. - * - * See: http://www.rfc-editor.org/errata_search.php?rfc=3986 + * Validate URIs using simplesamlphp/assert. * - * Note that this is a VERY permissive regex + * The legacy regex is kept for the unused parse() helper so existing debug + * output shape remains available if it is ever reintroduced. */ class EngineBlock_Validator_Uri { @@ -33,7 +35,13 @@ class EngineBlock_Validator_Uri */ public function validate($uri) { - return (bool) preg_match(self::REGEX, $uri); + try { + Assert::validURI($uri); + } catch (AssertionFailedException $e) { + return false; + } + + return true; } /** diff --git a/tests/library/EngineBlock/Test/Attributes/Validator/TypeTest.php b/tests/library/EngineBlock/Test/Attributes/Validator/TypeTest.php index 20fe9011d4..cd63cb8699 100644 --- a/tests/library/EngineBlock/Test/Attributes/Validator/TypeTest.php +++ b/tests/library/EngineBlock/Test/Attributes/Validator/TypeTest.php @@ -16,6 +16,7 @@ * limitations under the License. */ +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; class EngineBlock_Test_TypeTest extends TestCase @@ -26,13 +27,22 @@ class EngineBlock_Test_TypeTest extends TestCase * @param $options * @param $attributes */ - #[\PHPUnit\Framework\Attributes\DataProvider('validAttributesProvider')] + #[DataProvider('validAttributesProvider')] public function testAttributeValidates($attributeName, $options, $attributes) { $validator = new EngineBlock_Attributes_Validator_Type($attributeName, $options); $this->assertTrue($validator->validate($attributes)); } + #[DataProvider('invalidAttributesProvider')] + public function testAttributeValidationFails($attributeName, $options, $attributes, $expectedMessage) + { + $validator = new EngineBlock_Attributes_Validator_Type($attributeName, $options); + + $this->assertFalse($validator->validate($attributes)); + $this->assertSame([$expectedMessage, $attributeName, $options, $attributes[$attributeName][0]], $validator->getMessages()[0]); + } + public static function validAttributesProvider() { return array( @@ -59,7 +69,8 @@ public static function validAttributesProvider() 'options' => 'URI', 'attributes' => array( 'foo' => array( - '?' + '?', + 'urn:mace:dir:entitlement:common-lib-terms', ) ) ), @@ -78,4 +89,20 @@ public static function validAttributesProvider() ) ); } + + public static function invalidAttributesProvider() + { + return array( + array( + 'attributeName' => 'foo', + 'options' => 'URL', + 'attributes' => array( + 'foo' => array( + 'mailto:test@example.org', + ) + ), + 'expectedMessage' => EngineBlock_Attributes_Validator_Type::ERROR_ATTRIBUTE_VALIDATOR_URL, + ), + ); + } } diff --git a/tests/library/EngineBlock/Test/Validator/UriTest.php b/tests/library/EngineBlock/Test/Validator/UriTest.php index d44d365900..e6502e49b2 100644 --- a/tests/library/EngineBlock/Test/Validator/UriTest.php +++ b/tests/library/EngineBlock/Test/Validator/UriTest.php @@ -19,9 +19,6 @@ use Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration; use PHPUnit\Framework\TestCase; -/** - * @todo write test which tests failing...this validator is so permissive it is VERY hard to let it fail - */ class EngineBlock_Test_Validator_UriTest extends TestCase { use MockeryPHPUnitIntegration; @@ -46,7 +43,8 @@ public static function validUriProvider() { return array( array('http://example.com'), // Pretty standard http url - array('urn:mace:dir:entitlement:common-lib-terms') // Saml entitlement + array('urn:mace:dir:entitlement:common-lib-terms'), // Saml entitlement + array('?'), ); } }